Skip to content

Commit 87479aa

Browse files
jrburkejrburke
committed
Update optimizer to use the new exports vs return rules, specifically, do not add exports as a dependency if it is not part of the function arg list for CommonJS dependency scanning.
1 parent 7bf835e commit 87479aa

File tree

6 files changed

+52
-17
lines changed

6 files changed

+52
-17
lines changed

‎build/jslib/build.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -730,7 +730,7 @@ function (lang, logger, file, parse, optimize, pragma,
730730
return "'" + dep + "'";
731731
});
732732
} else {
733-
deps = null;
733+
deps = [];
734734
}
735735
}
736736

‎build/jslib/parse.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,15 +136,22 @@ define(['uglifyjs/index'], function (uglify) {
136136
parse.getAnonDeps = function (fileName, fileContents) {
137137
var astRoot = parser.parse(fileContents),
138138
deps = [],
139-
defFunc = this.findAnonRequireDefCallback(astRoot);
139+
defFunc = this.findAnonRequireDefCallback(astRoot),
140+
funcArgLength;
140141

141142
//Now look inside the def call's function for require calls.
142143
if (defFunc) {
143144
this.findRequireDepNames(defFunc, deps);
144145

145146
//If no deps, still add the standard CommonJS require, exports, module,
146-
//in that order, to the deps.
147-
deps = ["require", "exports", "module"].concat(deps);
147+
//in that order, to the deps, but only if specified as function args.
148+
//In particular, if exports is used, it is favored over the return
149+
//value of the function, so only add it if asked.
150+
funcArgLength = defFunc[2] && defFunc[2].length;
151+
if (funcArgLength) {
152+
deps = (funcArgLength > 1 ? ["require", "exports", "module"] :
153+
["require"]).concat(deps);
154+
}
148155
}
149156

150157
return deps;

‎build/tests/exports.build.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
//A simple build file using the tests directory for requirejs
2+
{
3+
baseUrl: "../../tests/exports",
4+
inlineText: false,
5+
dir: "builds/exports",
6+
optimize: "none",
7+
modules: [
8+
{
9+
name: "simpleReturn"
10+
}
11+
]
12+
}

‎build/tests/parse.js

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,21 @@ define(['parse'], function (parse) {
2929
good4 = '(function (require) { require.def("one", function(){}); }(myGlobalRequire))',
3030
bad1 = "require.def('one', [foo, 'me'], function() {});",
3131
bad2 = "require.def('one', somevar)",
32-
goodAnon1 = "require.def(function(){ var foo = require('foo'); });",
33-
goodAnon2 = "require.def(function () { if (true) { callback(function () { require(\"bar\"); })}});",
34-
emptyAnon1 = "require.def(function() { exports.name = 'empty'; });";
32+
goodAnon1 = "require.def(function(require){ var foo = require('foo'); });",
33+
goodAnon2 = "require.def(function (require, exports, module) { if (true) { callback(function () { require(\"bar\"); })}});",
34+
goodAnon3 = "require.def(function(require, exports, module) { exports.name = 'empty'; });",
35+
emptyAnon1 = "require.def(function(){ return 'foo'; });";
3536

3637
t.is('define("one",[ "two", "three" ]);', parse("good1", good1));
3738
t.is('define("one",function() {});', parse("good2", good2));
3839
t.is('define("one",[ "two" ]);', parse("good3", good3));
3940
t.is('define("one",function() {});', parse("good4", good4));
4041
t.is(null, parse("bad1", bad1));
4142
t.is(null, parse("bad2", bad2));
42-
t.is(['require', 'exports', 'module', 'foo'], parse.getAnonDeps("goodAnon1", goodAnon1));
43+
t.is(['require', 'foo'], parse.getAnonDeps("goodAnon1", goodAnon1));
4344
t.is(['require', 'exports', 'module', 'bar'], parse.getAnonDeps("goodAnon2", goodAnon2));
44-
t.is(3, parse.getAnonDeps("emptyAnon1", emptyAnon1).length);
45+
t.is(['require', 'exports', 'module'], parse.getAnonDeps("goodAnon3", goodAnon3));
46+
t.is(0, parse.getAnonDeps("emptyAnon1", emptyAnon1).length);
4547
}
4648
]
4749
);
@@ -59,9 +61,10 @@ define(['parse'], function (parse) {
5961
bad2 = "define('one', somevar)",
6062
bad3 = "function define(foo) { return foo };",
6163
bad4 = "define(a[0]);",
62-
goodAnon1 = "define(function(){ var foo = require('foo'); });",
63-
goodAnon2 = "define(function () { if (true) { callback(function () { require(\"bar\"); })}});",
64-
emptyAnon1 = "define(function() { exports.name = 'empty'; });";
64+
goodAnon1 = "define(function(require){ var foo = require('foo'); });",
65+
goodAnon2 = "define(function (require, exports, module) { if (true) { callback(function () { require(\"bar\"); })}});",
66+
goodAnon3 = "define(function(require, exports, module) { exports.name = 'empty'; });",
67+
emptyAnon1 = "define(function(){ return 'foo'; });";
6568

6669
t.is('define("one",[ "two", "three" ]);', parse("good1", good1));
6770
t.is('define("one",function() {});', parse("good2", good2));
@@ -73,10 +76,10 @@ define(['parse'], function (parse) {
7376
t.is(null, parse("bad3", bad3));
7477
t.is(null, parse("bad4", bad4));
7578

76-
77-
t.is(['require', 'exports', 'module', 'foo'], parse.getAnonDeps("goodAnon1", goodAnon1));
79+
t.is(['require', 'foo'], parse.getAnonDeps("goodAnon1", goodAnon1));
7880
t.is(['require', 'exports', 'module', 'bar'], parse.getAnonDeps("goodAnon2", goodAnon2));
79-
t.is(3, parse.getAnonDeps("emptyAnon1", emptyAnon1).length);
81+
t.is(['require', 'exports', 'module'], parse.getAnonDeps("goodAnon3", goodAnon3));
82+
t.is(0, parse.getAnonDeps("emptyAnon1", emptyAnon1).length);
8083
}
8184
]
8285
);

‎tests/exports/exports-tests.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
require({
22
baseUrl: require.isBrowser ? "./" : "./exports/"
33
},
4-
["require", "vanilla", "funcSet", "assign", "assign2", "usethis", "implicitModule"],
5-
function(require, vanilla, funcSet, assign, assign2, usethis, implicitModule) {
4+
["require", "vanilla", "funcSet", "assign", "assign2", "usethis",
5+
"implicitModule", "simpleReturn"],
6+
function(require, vanilla, funcSet, assign, assign2, usethis,
7+
implicitModule, simpleReturn) {
68
doh.register(
79
"exports",
810
[
@@ -13,6 +15,7 @@ require({
1315
t.is("assign2", assign2);
1416
t.is("usethis", usethis.name);
1517
t.is("implicitModule", implicitModule());
18+
t.is("simpleReturn", simpleReturn());
1619
}
1720
]
1821
);

‎tests/exports/simpleReturn.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
//This file does not use exports, just
2+
//return, but need to test that it does not
3+
//automatically get an exports object assigned
4+
define(
5+
function () {
6+
return function () {
7+
return 'simpleReturn';
8+
};
9+
}
10+
);

0 commit comments

Comments
 (0)