Skip to content

Commit

Permalink
Fixes #245, local require calls in a define are now preserved with th…
Browse files Browse the repository at this point in the history
…e namespace option.
  • Loading branch information
jrburke committed Dec 22, 2012
1 parent 679e66a commit e742f99
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 11 deletions.
46 changes: 40 additions & 6 deletions build/jslib/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@
define(['./esprima'], function (esprima) {
'use strict';

var ostring = Object.prototype.toString,
//This string is saved off because JSLint complains
//about obj.arguments use, as 'reserved word'
argPropName = 'arguments';
//This string is saved off because JSLint complains
//about obj.arguments use, as 'reserved word'
var argPropName = 'arguments';

//From an esprima example for traversing its ast.
function traverse(object, visitor) {
Expand Down Expand Up @@ -361,6 +360,41 @@ define(['./esprima'], function (esprima) {
}
};

/**
* Renames require/requirejs/define calls to be ns + '.' + require/requirejs/define
* Does *not* do .config calls though. See pragma.namespace for the complete
* set of namespace transforms. This function is used because require calls
* inside a define() call should not be renamed, so a simple regexp is not
* good enough.
* @param {String} fileContents the contents to transform.
* @param {String} ns the namespace, *not* including trailing dot.
* @return {String} the fileContents with the namespace applied
*/
parse.renameNamespace = function (fileContents, ns) {
var ranges = [],
astRoot = esprima.parse(fileContents, {
range: true
});

parse.recurse(astRoot, function (callName, config, name, deps, node) {
ranges.push(node.range);
//Do not recurse into define functions, they should be using
//local defines.
return callName !== 'define';
}, {});

//Go backwards through the found ranges, adding in the namespace name
//in front.
ranges.reverse();
ranges.forEach(function (range) {
fileContents = fileContents.substring(0, range[0]) +
ns + '.' +
fileContents.substring(range[0]);
});

return fileContents;
};

/**
* Finds all dependencies specified in dependency arrays and inside
* simplified commonjs wrappers.
Expand Down Expand Up @@ -619,7 +653,7 @@ define(['./esprima'], function (esprima) {
return;
}

return onMatch("require", null, null, deps);
return onMatch("require", null, null, deps, node);
} else if (parse.hasDefine(node) && args && args.length) {
name = args[0];
deps = args[1];
Expand Down Expand Up @@ -669,7 +703,7 @@ define(['./esprima'], function (esprima) {
name = name.value;
}

return onMatch("define", null, name, deps);
return onMatch("define", null, name, deps, node);
}
};

Expand Down
7 changes: 5 additions & 2 deletions build/jslib/pragma.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ define(['parse', 'logger'], function (parse, logger) {
conditionalRegExp: /(exclude|include)Start\s*\(\s*["'](\w+)["']\s*,(.*)\)/,
useStrictRegExp: /['"]use strict['"];/g,
hasRegExp: /has\s*\(\s*['"]([^'"]+)['"]\s*\)/g,
nsRegExp: /(^|[^\.])(requirejs|require|define)(\.config)?\s*\(/g,
configRegExp: /(^|[^\.])(requirejs|require)(\.config)\s*\(/g,
nsWrapRegExp: /\/\*requirejs namespace: true \*\//,
apiDefRegExp: /var requirejs, require, define;/,
defineCheckRegExp: /typeof\s+define\s*===\s*["']function["']\s*&&\s*define\s*\.\s*amd/g,
Expand All @@ -50,7 +50,10 @@ define(['parse', 'logger'], function (parse, logger) {
namespace: function (fileContents, ns, onLifecycleName) {
if (ns) {
//Namespace require/define calls
fileContents = fileContents.replace(pragma.nsRegExp, '$1' + ns + '.$2$3(');
fileContents = fileContents.replace(pragma.configRegExp, '$1' + ns + '.$2$3(');


fileContents = parse.renameNamespace(fileContents, ns);

//Namespace define ternary use:
fileContents = fileContents.replace(pragma.defineTernaryRegExp,
Expand Down
6 changes: 3 additions & 3 deletions build/tests/lib/namespace/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ if ('function' === typeof foo.define && foo.define.amd) {
}
;
(function (define) {
foo.define('modules/three',['require'], function (require) {
foo.define('modules/three',['require','./four','./five'], function (require) {
//If have dependencies, get them here
var four = foo.require('./four'),
five = foo.require('./five');
var four = require('./four'),
five = require('./five');

//Return the module definition.
return {
Expand Down

0 comments on commit e742f99

Please sign in to comment.