Skip to content

Commit

Permalink
Fixes #3187 (couldn't repo, but found bugs) (#3229)
Browse files Browse the repository at this point in the history 
* (Maybe) Fixes #3187 (couldn't repo, but found bug)
- Fixes multiple Node.js plugins not being loaded
* Added .eslintignore
* Added tests to fix #3187
* Fixed browser test
  • Loading branch information
@matthew-dean
matthew-dean authored Jun 30, 2018
1 parent f1a3a25 commit 7a47223
Show file tree
Hide file tree
Showing 13 changed files with 358 additions and 122 deletions.
1 change: 1 addition & 0 deletions .eslintignore
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Gruntfile.js
5 changes: 4 additions & 1 deletion Gruntfile.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,10 @@ module.exports = function (grunt) {
command: [
'node bin/lessc --clean-css="--s1 --advanced" test/less/lazy-eval.less tmp/lazy-eval.css',
'cd lib',
'node ../bin/lessc --clean-css="--s1 --advanced" ../test/less/lazy-eval.less ../tmp/lazy-eval.css'
'node ../bin/lessc --clean-css="--s1 --advanced" ../test/less/lazy-eval.less ../tmp/lazy-eval.css',
'cd ..',
// Test multiple plugins
'node bin/lessc --plugin=clean-css="--s1 --advanced" --plugin=autoprefix="ie 11,Edge >= 13,Chrome >= 47,Firefox >= 45,iOS >= 9.2,Safari >= 9" test/less/lazy-eval.less tmp/lazy-eval.css'
].join(' && ')
},
'sourcemap-test': {
Expand Down
86 changes: 50 additions & 36 deletions dist/less.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -886,8 +886,7 @@ var AbstractPluginLoader = require('../less/environment/abstract-plugin-loader.j
*/
var PluginLoader = function(less) {
this.less = less;
// shim for browser require?
this.require = require;
// Should we shim this.require for browser? Probably not?
};

PluginLoader.prototype = new AbstractPluginLoader();
Expand Down Expand Up @@ -1436,24 +1435,21 @@ var functionRegistry = require('../functions/function-registry'),
LessError = require('../less-error');

var AbstractPluginLoader = function() {
// Implemented by Node.js plugin loader
this.require = function() {
return null;
}
};

function error(msg, type) {
throw new LessError(
{
type: type || 'Syntax',
message: msg
}
);
}
AbstractPluginLoader.prototype.evalPlugin = function(contents, context, imports, pluginOptions, fileInfo) {

var loader,
registry,
pluginObj,
localModule,
pluginManager,
filename;
filename,
result;

pluginManager = context.pluginManager;

Expand All @@ -1471,15 +1467,18 @@ AbstractPluginLoader.prototype.evalPlugin = function(contents, context, imports,
pluginObj = pluginManager.get(filename);

if (pluginObj) {
this.trySetOptions(pluginObj, filename, shortname, pluginOptions);
result = this.trySetOptions(pluginObj, filename, shortname, pluginOptions);
if (result) {
return result;
}
try {
if (pluginObj.use) {
pluginObj.use.call(this.context, pluginObj);
}
}
catch (e) {
e.message = 'Error during @plugin call';
return new this.less.LessError(e, imports, filename);
e.message = e.message || 'Error during @plugin call';
return new LessError(e, imports, filename);
}
return pluginObj;
}
Expand All @@ -1497,24 +1496,38 @@ AbstractPluginLoader.prototype.evalPlugin = function(contents, context, imports,

try {
loader = new Function('module', 'require', 'registerPlugin', 'functions', 'tree', 'less', 'fileInfo', contents);
loader(localModule, this.require, registerPlugin, registry, this.less.tree, this.less, fileInfo);
loader(localModule, this.require(filename), registerPlugin, registry, this.less.tree, this.less, fileInfo);
} catch (e) {
return new this.less.LessError(e, imports, filename);
return new LessError(e, imports, filename);
}

if (!pluginObj) {
pluginObj = localModule.exports;
}
pluginObj = this.validatePlugin(pluginObj, filename, shortname);

if (pluginObj instanceof LessError) {
return pluginObj;
}

if (pluginObj) {
// For 2.x back-compatibility - setOptions() before install()
pluginObj.imports = imports;
pluginObj.filename = filename;
result = this.trySetOptions(pluginObj, filename, shortname, pluginOptions);
if (result) {
return result;
}

// Run on first load
pluginManager.addPlugin(pluginObj, fileInfo.filename, registry);
pluginObj.functions = registry.getLocalFunctions();
pluginObj.imports = imports;
pluginObj.filename = filename;

this.trySetOptions(pluginObj, filename, shortname, pluginOptions);
// Need to call setOptions again because the pluginObj might have functions
result = this.trySetOptions(pluginObj, filename, shortname, pluginOptions);
if (result) {
return result;
}

// Run every @plugin call
try {
Expand All @@ -1523,32 +1536,31 @@ AbstractPluginLoader.prototype.evalPlugin = function(contents, context, imports,
}
}
catch (e) {
e.message = 'Error during @plugin call';
return new this.less.LessError(e, imports, filename);
e.message = e.message || 'Error during @plugin call';
return new LessError(e, imports, filename);
}

}
else {
return new this.less.LessError({ message: 'Not a valid plugin' });
return new LessError({ message: 'Not a valid plugin' }, imports, filename);
}

return pluginObj;

};

AbstractPluginLoader.prototype.trySetOptions = function(plugin, filename, name, options) {
if (options) {
if (!plugin.setOptions) {
error('Options have been provided but the plugin ' + name + ' does not support any options.');
return null;
}
try {
plugin.setOptions(options);
}
catch (e) {
error('Error setting options on plugin ' + name + '\n' + e.message);
return null;
}
if (options && !plugin.setOptions) {
return new LessError({
message: 'Options have been provided but the plugin ' +
name + ' does not support any options.'
});
}
try {
plugin.setOptions && plugin.setOptions(options);
}
catch (e) {
return new LessError(e);
}
};

Expand All @@ -1562,8 +1574,10 @@ AbstractPluginLoader.prototype.validatePlugin = function(plugin, filename, name)

if (plugin.minVersion) {
if (this.compareVersion(plugin.minVersion, this.less.version) < 0) {
error('Plugin ' + name + ' requires version ' + this.versionToString(plugin.minVersion));
return null;
return new LessError({
message: 'Plugin ' + name + ' requires version ' +
this.versionToString(plugin.minVersion)
});
}
}
return plugin;
Expand Down
2 changes: 1 addition & 1 deletion dist/less.min.js
View file

Large diffs are not rendered by default.

3 changes: 1 addition & 2 deletions lib/less-browser/plugin-loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ var AbstractPluginLoader = require('../less/environment/abstract-plugin-loader.j
*/
var PluginLoader = function(less) {
this.less = less;
// shim for browser require?
this.require = require;
// Should we shim this.require for browser? Probably not?
};

PluginLoader.prototype = new AbstractPluginLoader();
Expand Down
5 changes: 1 addition & 4 deletions lib/less-node/plugin-loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ var path = require('path'),
*/
var PluginLoader = function(less) {
this.less = less;
this.require = require;
this.requireRelative = function(prefix) {
this.require = function(prefix) {
prefix = path.dirname(prefix);
return function(id) {
var str = id.substr(0, 2);
Expand All @@ -25,7 +24,6 @@ var PluginLoader = function(less) {
PluginLoader.prototype = new AbstractPluginLoader();

PluginLoader.prototype.loadPlugin = function(filename, basePath, context, environment, fileManager) {
var self = this;
var prefix = filename.slice(0, 1);
var explicit = prefix === '.' || prefix === '/' || filename.slice(-3).toLowerCase() === '.js';
if (!explicit) {
Expand All @@ -36,7 +34,6 @@ PluginLoader.prototype.loadPlugin = function(filename, basePath, context, enviro
fileManager.loadFile(filename, basePath, context, environment).then(
function(data) {
try {
self.require = self.requireRelative(data.filename);
fulfill(data);
}
catch (e) {
Expand Down
83 changes: 49 additions & 34 deletions lib/less/environment/abstract-plugin-loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,21 @@ var functionRegistry = require('../functions/function-registry'),
LessError = require('../less-error');

var AbstractPluginLoader = function() {
// Implemented by Node.js plugin loader
this.require = function() {
return null;
}
};

function error(msg, type) {
throw new LessError(
{
type: type || 'Syntax',
message: msg
}
);
}
AbstractPluginLoader.prototype.evalPlugin = function(contents, context, imports, pluginOptions, fileInfo) {

var loader,
registry,
pluginObj,
localModule,
pluginManager,
filename;
filename,
result;

pluginManager = context.pluginManager;

Expand All @@ -37,15 +34,18 @@ AbstractPluginLoader.prototype.evalPlugin = function(contents, context, imports,
pluginObj = pluginManager.get(filename);

if (pluginObj) {
this.trySetOptions(pluginObj, filename, shortname, pluginOptions);
result = this.trySetOptions(pluginObj, filename, shortname, pluginOptions);
if (result) {
return result;
}
try {
if (pluginObj.use) {
pluginObj.use.call(this.context, pluginObj);
}
}
catch (e) {
e.message = 'Error during @plugin call';
return new this.less.LessError(e, imports, filename);
e.message = e.message || 'Error during @plugin call';
return new LessError(e, imports, filename);
}
return pluginObj;
}
Expand All @@ -63,24 +63,38 @@ AbstractPluginLoader.prototype.evalPlugin = function(contents, context, imports,

try {
loader = new Function('module', 'require', 'registerPlugin', 'functions', 'tree', 'less', 'fileInfo', contents);
loader(localModule, this.require, registerPlugin, registry, this.less.tree, this.less, fileInfo);
loader(localModule, this.require(filename), registerPlugin, registry, this.less.tree, this.less, fileInfo);
} catch (e) {
return new this.less.LessError(e, imports, filename);
return new LessError(e, imports, filename);
}

if (!pluginObj) {
pluginObj = localModule.exports;
}
pluginObj = this.validatePlugin(pluginObj, filename, shortname);

if (pluginObj instanceof LessError) {
return pluginObj;
}

if (pluginObj) {
// For 2.x back-compatibility - setOptions() before install()
pluginObj.imports = imports;
pluginObj.filename = filename;
result = this.trySetOptions(pluginObj, filename, shortname, pluginOptions);
if (result) {
return result;
}

// Run on first load
pluginManager.addPlugin(pluginObj, fileInfo.filename, registry);
pluginObj.functions = registry.getLocalFunctions();
pluginObj.imports = imports;
pluginObj.filename = filename;

this.trySetOptions(pluginObj, filename, shortname, pluginOptions);
// Need to call setOptions again because the pluginObj might have functions
result = this.trySetOptions(pluginObj, filename, shortname, pluginOptions);
if (result) {
return result;
}

// Run every @plugin call
try {
Expand All @@ -89,32 +103,31 @@ AbstractPluginLoader.prototype.evalPlugin = function(contents, context, imports,
}
}
catch (e) {
e.message = 'Error during @plugin call';
return new this.less.LessError(e, imports, filename);
e.message = e.message || 'Error during @plugin call';
return new LessError(e, imports, filename);
}

}
else {
return new this.less.LessError({ message: 'Not a valid plugin' });
return new LessError({ message: 'Not a valid plugin' }, imports, filename);
}

return pluginObj;

};

AbstractPluginLoader.prototype.trySetOptions = function(plugin, filename, name, options) {
if (options) {
if (!plugin.setOptions) {
error('Options have been provided but the plugin ' + name + ' does not support any options.');
return null;
}
try {
plugin.setOptions(options);
}
catch (e) {
error('Error setting options on plugin ' + name + '\n' + e.message);
return null;
}
if (options && !plugin.setOptions) {
return new LessError({
message: 'Options have been provided but the plugin ' +
name + ' does not support any options.'
});
}
try {
plugin.setOptions && plugin.setOptions(options);
}
catch (e) {
return new LessError(e);
}
};

Expand All @@ -128,8 +141,10 @@ AbstractPluginLoader.prototype.validatePlugin = function(plugin, filename, name)

if (plugin.minVersion) {
if (this.compareVersion(plugin.minVersion, this.less.version) < 0) {
error('Plugin ' + name + ' requires version ' + this.versionToString(plugin.minVersion));
return null;
return new LessError({
message: 'Plugin ' + name + ' requires version ' +
this.versionToString(plugin.minVersion)
});
}
}
return plugin;
Expand Down
Loading

0 comments on commit 7a47223

Please sign in to comment.