diff --git a/packages/coffeescript/plugin/compile-coffeescript.js b/packages/coffeescript/plugin/compile-coffeescript.js index 06e1626f03..51c09695ee 100644 --- a/packages/coffeescript/plugin/compile-coffeescript.js +++ b/packages/coffeescript/plugin/compile-coffeescript.js @@ -1,6 +1,74 @@ var fs = Npm.require('fs'); var path = Npm.require('path'); var coffee = Npm.require('coffee-script'); +var _ = Npm.require('underscore'); + +var stripExportedVars = function (source, exports) { + if (!exports || _.isEmpty(exports)) + return source; + var lines = source.split("\n"); + + // We make the following assumptions, based on the output of CoffeeScript + // 1.6.3. + // - The var declaration in question is not indented and is the first such + // var declaration. (CoffeeScript only produces one var line at each + // scope and there's only one top-level scope.) All relevant variables + // are actually on this line. + // - The user hasn't used a ###-comment containing a line that looks like + // a var line, to produce something like + // /* bla + // var foo; + // */ + // before an actual var line. (ie, we do NOT attempt to figure out if + // we're inside a /**/ comment, which is produced by ### comments.) + // - The var in question is not assigned to in the declaration, nor are any + // other vars on this line. (CoffeeScript does produce some assignments + // but only for internal helpers generated by CoffeeScript, and they end + // up on subsequent lines.) + // XXX relax these assumptions by doing actual JS parsing (eg with jsparse). + // I'd do this now, but there's no easy way to "unparse" a jsparse AST. + + var foundVarLine = false; + lines = _.map(lines, function (line) { + if (foundVarLine) + return line; + var match = /^var (.+)([,;])$/.exec(line); + if (!match) + return line; + foundVarLine = true; + + // If there's an assignment on this line, we assume that there are ONLY + // assignments and that the var we are looking for is not declared. (Part + // of our strong assumption about the layout of this code.) + if (match[1].indexOf('=') !== -1) + return line; + + var vars = match[1].split(', '); + vars = _.difference(vars, exports); + if (!_.isEmpty(vars)) + return "var " + vars.join(', ') + match[2]; + // We got rid of all the vars on this line. Drop the whole line if this + // didn't continue to the next line, otherwise keep just the 'var '. + return match[2] === ';' ? '' : 'var'; + }); + return lines.join('\n'); +}; + +var addSharedHeader = function (source) { + // We want the symbol "share" to be visible to all CoffeeScript files in the + // package (and shared between them), but not visible to JavaScript + // files. (That's because we don't want to introduce two competing ways to + // make package-local variables into JS ("share" vs assigning to non-var + // variables).) The following hack accomplishes that: "__coffeescriptShare" + // will be visible at the package level and "share" at the file level. This + // should work both in "package" mode where __coffeescriptShare will be added + // as a var in the package closure, and in "app" mode where it will end up as + // a global. + return ("__coffeescriptShare = typeof __coffeescriptShare === 'object' ? " + + "__coffeescriptShare : {}; " + + "var share = __coffeescriptShare;" + + source); +}; var handler = function (compileStep) { var source = compileStep.read().toString('utf8'); @@ -21,27 +89,14 @@ var handler = function (compileStep) { ); } - // We want the symbol "share" to be visible to all CoffeeScript files in the - // package (and shared between them), but not visible to JavaScript - // files. (That's because we don't want to introduce two competing ways to - // make package-local variables into JS ("share" vs assigning to non-var - // variables).) The following hack accomplishes that: "__coffeescriptShare" - // will be visible at the package level and "share" at the file level. This - // should work both in "package" mode where __coffeescriptShare will be added - // as a var in the package closure, and in "app" mode where it will end up as - // a global. - // - // We need a newline after this (which may require a source map to be - // adjusted), to not conflict with stripVarFromExports. - output = ("__coffeescriptShare = typeof __coffeescriptShare === 'object' ? __coffeescriptShare : {}; " + - "var share = __coffeescriptShare;\n" + output); - compileStep.addJavaScript({ path: compileStep.inputPath + ".js", sourcePath: compileStep.inputPath, data: output, lineForLine: false, - stripVarFromExports: true + linkerUnitTransform: function (source, exports) { + return addSharedHeader(stripExportedVars(source, exports)); + } }); }; diff --git a/tools/linker.js b/tools/linker.js index f85099e033..b188680254 100644 --- a/tools/linker.js +++ b/tools/linker.js @@ -54,10 +54,10 @@ _.extend(Module.prototype, { // source: the source code // servePath: the path where it would prefer to be served if possible addFile: function (source, servePath, sourcePath, includePositionInErrors, - stripVarFromExports) { + linkerUnitTransform) { var self = this; self.files.push(new File(source, servePath, sourcePath, - includePositionInErrors, stripVarFromExports)); + includePositionInErrors, linkerUnitTransform)); }, @@ -257,7 +257,7 @@ var writeSymbolTree = function (symbolTree, indent) { /////////////////////////////////////////////////////////////////////////////// var File = function (source, servePath, sourcePath, includePositionInErrors, - stripVarFromExports) { + linkerUnitTransform) { var self = this; // source code for this file (a string) @@ -276,9 +276,11 @@ var File = function (source, servePath, sourcePath, includePositionInErrors, // the source of each unit, in order, will give self.source. self.units = []; - // Should we try to ensure that exports are not also declared as top-level - // vars? (eg, for CoffeeScript.) - self.stripVarFromExports = stripVarFromExports; + // A function which transforms the source code once all exports are + // known. (eg, for CoffeeScript.) + self.linkerUnitTransform = linkerUnitTransform || function (source, exports) { + return source; + }; self._unitize(); }; @@ -302,7 +304,7 @@ _.extend(File.prototype, { // numbers don't change between input and output. In this case, // sourceWidth is ignored. // - sourceWidth: width in columns to use for the source code - // - exports: the exports to remove, if stripVarFromExports was set + // - exports: the module's exports getLinkedOutput: function (options) { var self = this; @@ -312,7 +314,7 @@ _.extend(File.prototype, { if (options.preserveLineNumbers) { // Ugly version return "(function(){" + - self._maybeStripVars(self.source, options.exports) + "\n})();\n"; + self.linkerUnitTransform(self.source, options.exports) + "\n})();\n"; } // Pretty version @@ -340,7 +342,7 @@ _.extend(File.prototype, { // already inside a comment. var num = 1; _.each(self.units, function (unit) { - var unitSource = self._maybeStripVars(unit.source, options.exports); + var unitSource = self.linkerUnitTransform(unit.source, options.exports); var lines = unitSource.split('\n'); _.each(lines, function (line) { @@ -432,59 +434,6 @@ _.extend(File.prototype, { buf += line; }); unit.source = buf; - }, - - _maybeStripVars: function (source, exports) { - var self = this; - - if (!self.stripVarFromExports || !exports || _.isEmpty(exports)) - return source; - var lines = source.split("\n"); - - // We make the following assumptions, based on the output of CoffeeScript - // 1.6.3. - // - The var declaration in question is not indented and is the first such - // var declaration. (CoffeeScript only produces one var line at each - // scope and there's only one top-level scope.) All relevant variables - // are actually on this line. - // - The user hasn't used a ###-comment containing a line that looks like - // a var line, to produce something like - // /* bla - // var foo; - // */ - // before an actual var line. (ie, we do NOT attempt to figure out if - // we're inside a /**/ comment, which is produced by ### comments.) - // - The var in question is not assigned to in the declaration, nor are - // any other vars on this line. (CoffeeScript does produce some - // assignments but only for internal helpers generated by CoffeeScript, - // and they end up on subsequent lines.) - // XXX relax these assumptions by doing actual JS parsing (eg with jsparse). - // I'd do this now, but there's no easy way to "unparse" a jsparse AST. - - var foundVarLine = false; - lines = _.map(lines, function (line) { - if (foundVarLine) - return line; - var match = /^var (.+)([,;])$/.exec(line); - if (!match) - return line; - foundVarLine = true; - - // If there's an assignment on this line, we assume that there are ONLY - // assignments and that the var we are looking for is not declared. (Part - // of our strong assumption about the layout of this code.) - if (match[1].indexOf('=') !== -1) - return line; - - var vars = match[1].split(', '); - vars = _.difference(vars, exports); - if (!_.isEmpty(vars)) - return "var " + vars.join(', ') + match[2]; - // We got rid of all the vars on this line. Drop the whole line if this - // didn't continue to the next line, otherwise keep just the 'var '. - return match[2] === ';' ? '' : 'var'; - }); - return lines.join('\n'); } }); @@ -612,10 +561,10 @@ _.extend(Unit.prototype, { // - includePositionInErrors: true to include line and column // information in errors. set to false if, eg, this is the output // of coffeescript. (XXX replace with real sourcemaps) -// - stripVarFromExports: true if we should look at the top of the file -// for a "var" declaration which may declare some exports as unassigned -// vars, and if so, remove the var declaration. (This is designed -// to work with the output of the CoffeeScript compiler.) +// - linkerUnitTransform: if given, this function will be called +// when the module is being linked with the source of the unit +// and an array of the exports of the module; the unit's source will +// be replaced by what the function returns. // // forceExport: an array of symbols (as dotted strings) to force the // module to export, even if it wouldn't otherwise @@ -648,7 +597,7 @@ var prelink = function (options) { _.each(options.inputFiles, function (f) { module.addFile(f.source, f.servePath, f.sourcePath, f.includePositionInErrors, - f.stripVarFromExports); + f.linkerUnitTransform); }); var files = module.getLinkedFiles(); diff --git a/tools/packages.js b/tools/packages.js index 4f3360a244..307e0356c5 100644 --- a/tools/packages.js +++ b/tools/packages.js @@ -372,7 +372,7 @@ _.extend(Slice.prototype, { sourcePath: options.sourcePath, servePath: path.join(self.pkg.serveRoot, options.path), includePositionInErrors: options.lineForLine, - stripVarFromExports: !!options.stripVarFromExports + linkerUnitTransform: options.linkerUnitTransform }); }, addAsset: function (options) {