From c3d1f7bbd221e2877c35b050ce526583ae2ff667 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Fri, 2 Aug 2013 15:16:53 -0700 Subject: [PATCH] Don't fail if stylus or less files are in a non-client-specific place. Make the plugin's arch check less sketchy. Improve less and stylus error handling. --- packages/less/plugin/compile-less.js | 38 +++++++++++++++------- packages/meteor/plugin/basic-file-types.js | 7 ++-- packages/stylus/package.js | 2 +- packages/stylus/plugin/compile-stylus.js | 33 +++++++++++++------ tools/packages.js | 3 ++ 5 files changed, 57 insertions(+), 26 deletions(-) diff --git a/packages/less/plugin/compile-less.js b/packages/less/plugin/compile-less.js index cee7fc5e7e..41980738a8 100644 --- a/packages/less/plugin/compile-less.js +++ b/packages/less/plugin/compile-less.js @@ -1,8 +1,18 @@ var fs = Npm.require('fs'); var path = Npm.require('path'); var less = Npm.require('less'); +var Future = Npm.require('fibers/future'); Plugin.registerSourceHandler("less", function (compileStep) { + // XXX annoying that this is replicated in .css, .less, and .styl + if (! compileStep.archMatches('browser')) { + // XXX in the future, might be better to emit some kind of a + // warning if a stylesheet is included on the server, rather than + // silently ignoring it. but that would mean you can't stick .css + // at the top level of your app, which is kind of silly. + return; + } + var source = compileStep.read().toString('utf8'); var options = { // Use fs.readFileSync to process @imports. This is the bundler, so @@ -13,24 +23,28 @@ Plugin.registerSourceHandler("less", function (compileStep) { paths: [path.dirname(compileStep._fullInputPath)] // for @import }; + var f = new Future; + var css; try { - less.render(source, options, function (err, css) { - if (err) { - // XXX better error handling, once the Plugin interface support it - throw new Error(err.message); - } - - compileStep.addStylesheet({ - path: compileStep.inputPath + ".css", - data: css - }); - }); + less.render(source, options, f.resolver()); + css = f.wait(); } catch (e) { // less.render() is supposed to report any errors via its // callback. But sometimes, it throws them instead. This is // probably a bug in less. Be prepared for either behavior. - throw new Error(source_path + ": Less compiler error: " + e.message); + compileStep.error({ + message: "Less compiler error: " + e.message, + sourcePath: e.filename || compileStep.inputPath, + line: e.line - 1, // dunno why, but it matches + column: e.column + 1 + }); + return; } + + compileStep.addStylesheet({ + path: compileStep.inputPath + ".css", + data: css + }); });; // Register lessimport files with the dependency watcher, without actually diff --git a/packages/meteor/plugin/basic-file-types.js b/packages/meteor/plugin/basic-file-types.js index 14141699a3..3c87328198 100644 --- a/packages/meteor/plugin/basic-file-types.js +++ b/packages/meteor/plugin/basic-file-types.js @@ -3,11 +3,12 @@ source file. */ Plugin.registerSourceHandler("css", function (compileStep) { - // XXX use archinfo rather than rolling our own - if (! compileStep.arch.match(/^browser(\.|$)/)) { + // XXX annoying that this is replicated in .css, .less, and .styl + if (! compileStep.archMatches('browser')) { // XXX in the future, might be better to emit some kind of a // warning if a stylesheet is included on the server, rather than - // silently ignoring it + // silently ignoring it. but that would mean you can't stick .css + // at the top level of your app, which is kind of silly. return; } diff --git a/packages/stylus/package.js b/packages/stylus/package.js index 95e74bac9f..850d7cd49a 100644 --- a/packages/stylus/package.js +++ b/packages/stylus/package.js @@ -12,7 +12,7 @@ Package._transitional_registerBuildPlugin({ }); Package.on_test(function (api) { - api.use(['tinytest', 'stylus', 'test-helpers']) + api.use(['tinytest', 'stylus', 'test-helpers']); api.use('spark'); api.add_files(['stylus_tests.styl', 'stylus_tests.js'], 'client'); }); diff --git a/packages/stylus/plugin/compile-stylus.js b/packages/stylus/plugin/compile-stylus.js index 74b786d81d..7ddc33dcf5 100644 --- a/packages/stylus/plugin/compile-stylus.js +++ b/packages/stylus/plugin/compile-stylus.js @@ -1,21 +1,34 @@ var fs = Npm.require('fs'); var stylus = Npm.require('stylus'); var nib = Npm.require('nib'); +var Future = Npm.require('fibers/future'); Plugin.registerSourceHandler("styl", function (compileStep) { + // XXX annoying that this is replicated in .css, .less, and .styl + if (! compileStep.archMatches('browser')) { + // XXX in the future, might be better to emit some kind of a + // warning if a stylesheet is included on the server, rather than + // silently ignoring it. but that would mean you can't stick .css + // at the top level of your app, which is kind of silly. + return; + } + + var f = new Future; stylus(compileStep.read().toString('utf8')) .use(nib()) .set('filename', compileStep.inputPath) - .render(function(err, output) { - if (err) { - // XXX better error handling, once the Plugin interface support it - throw new Error('Stylus compiler error: ' + err.message); - } + .render(f.resolver()); - compileStep.addStylesheet({ - path: compileStep.inputPath + ".css", - data: output - }); + try { + var css = f.wait(); + } catch (e) { + compileStep.error({ + message: "Stylus compiler error: " + e.message }); + return; } -); + compileStep.addStylesheet({ + path: compileStep.inputPath + ".css", + data: css + }); +}); diff --git a/tools/packages.js b/tools/packages.js index baa536c19b..707efade99 100644 --- a/tools/packages.js +++ b/tools/packages.js @@ -365,6 +365,9 @@ _.extend(Slice.prototype, { packageName: self.pkg.name, rootOutputPath: self.pkg.serveRoot, arch: self.arch, + archMatches: function (pattern) { + return archinfo.matches(self.arch, pattern); + }, fileOptions: fileOptions, declaredExports: _.pluck(self.declaredExports, 'name'), read: function (n) {