Skip to content

Commit

Permalink
Merge pull request #2522 from rjgotten/fix-plugin-scoping
Browse files Browse the repository at this point in the history
Fix `@plugin` scoping rules
  • Loading branch information
lukeapage committed Apr 1, 2015
2 parents ecd3f66 + aa66271 commit 86fa4f6
Show file tree
Hide file tree
Showing 18 changed files with 193 additions and 49 deletions.
18 changes: 11 additions & 7 deletions lib/less/tree/import.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,11 @@ Import.prototype.isVariableImport = function () {
};
Import.prototype.evalForImport = function (context) {
var path = this.path;

if (path instanceof URL) {
path = path.value;
}

return new Import(path.eval(context), this.features, this.options, this.index, this.currentFileInfo);
};
Import.prototype.evalPath = function (context) {
Expand All @@ -108,6 +110,14 @@ Import.prototype.eval = function (context) {
var ruleset, registry,
features = this.features && this.features.eval(context);

if (this.options.plugin) {
registry = context.frames[0] && context.frames[0].functionRegistry;
if ( registry && this.root && this.root.functions ) {
registry.addMultiple( this.root.functions );
}
return [];
}

if (this.skip) {
if (typeof this.skip === "function") {
this.skip = this.skip();
Expand All @@ -117,13 +127,7 @@ Import.prototype.eval = function (context) {
}
}

if (this.options.plugin) {
registry = context.frames[0] && context.frames[0].functionRegistry;
if ( registry && this.root.functions ) {
registry.addMultiple( this.root.functions );
}
return [];
} else if (this.options.inline) {
if (this.options.inline) {
var contents = new Anonymous(this.root, 0, {filename: this.importedFilename}, true, true);
return this.features ? new Media([contents], this.features.value) : [contents];
} else if (this.css) {
Expand Down
1 change: 1 addition & 0 deletions lib/less/tree/media.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ Media.prototype.eval = function (context) {
context.mediaPath.push(media);
context.mediaBlocks.push(media);

this.rules[0].functionRegistry = context.frames[0].functionRegistry.inherit();
context.frames.unshift(this.rules[0]);
media.rules = [this.rules[0].eval(context)];
context.frames.shift();
Expand Down
4 changes: 3 additions & 1 deletion lib/less/tree/mixin-definition.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,10 @@ Definition.prototype.evalParams = function (context, mixinEnv, args, evaldArgume
params = this.params.slice(0),
i, j, val, name, isNamedFound, argIndex, argsLength = 0;

if (mixinEnv.frames && mixinEnv.frames[0] && mixinEnv.frames[0].functionRegistry) {
frame.functionRegistry = mixinEnv.frames[0].functionRegistry.inherit();
}
mixinEnv = new contexts.Eval(mixinEnv, [frame].concat(mixinEnv.frames));
frame.functionRegistry = context.frames[0].functionRegistry.inherit();

if (args) {
args = args.slice(0);
Expand Down
11 changes: 10 additions & 1 deletion lib/less/tree/ruleset.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,16 @@ Ruleset.prototype.eval = function (context) {

// inherit a function registry from the frames stack when possible;
// otherwise from the global registry
ruleset.functionRegistry = ((context.frames[0] && context.frames[0].functionRegistry) || globalFunctionRegistry).inherit();
ruleset.functionRegistry = (function (frames) {
var i = 0,
n = frames.length,
found;
for ( ; i !== n ; ++i ) {
found = frames[ i ].functionRegistry;
if ( found ) { return found; }
}
return globalFunctionRegistry;
}(context.frames)).inherit();

// push the current ruleset to the frames stack
var ctxFrames = context.frames;
Expand Down
14 changes: 12 additions & 2 deletions lib/less/visitors/import-visitor.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ ImportVisitor.prototype = {

var importVisitor = this,
inlineCSS = importNode.options.inline,
isPlugin = importNode.options.plugin,
duplicateImport = importedAtRoot || fullPath in importVisitor.recursionDetector;

if (!context.importMultiple) {
Expand All @@ -123,7 +124,7 @@ ImportVisitor.prototype = {
importNode.root = root;
importNode.importedFilename = fullPath;

if (!inlineCSS && (context.importMultiple || !duplicateImport)) {
if (!inlineCSS && !isPlugin && (context.importMultiple || !duplicateImport)) {
importVisitor.recursionDetector[fullPath] = true;

var oldContext = this.context;
Expand All @@ -144,7 +145,16 @@ ImportVisitor.prototype = {
}
},
visitRule: function (ruleNode, visitArgs) {
visitArgs.visitDeeper = false;
if (ruleNode.value.type === "DetachedRuleset") {
this.context.frames.unshift(ruleNode);
} else {
visitArgs.visitDeeper = false;
}
},
visitRuleOut : function(ruleNode) {
if (ruleNode.value.type === "DetachedRuleset") {
this.context.frames.shift();
}
},
visitDirective: function (directiveNode, visitArgs) {
this.context.frames.unshift(directiveNode);
Expand Down
6 changes: 0 additions & 6 deletions test/css/import-plugin-scoped.css

This file was deleted.

6 changes: 0 additions & 6 deletions test/css/import-plugin-tiered.css

This file was deleted.

3 changes: 0 additions & 3 deletions test/css/import-plugin.css

This file was deleted.

44 changes: 44 additions & 0 deletions test/css/plugin.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
.other {
trans: transitive;
}
.class {
trans: transitive;
global: global;
local: test-local();
shadow: global;
}
.class .local {
global: global;
local: local;
shadow: local;
}
.class {
ns-mixin-global: global;
ns-mixin-local: local;
ns-mixin-shadow: local;
mixin-local: local;
mixin-global: global;
mixin-shadow: local;
ruleset-local: local;
ruleset-global: global;
ruleset-shadow: local;
class-local: test-local();
}
@media screen {
.test {
result: global;
}
}
@font-face {
result: global;
}
@media screen and (min-width: 100px) and (max-width: 400px) {
.test {
result: global;
}
}
@media screen {
.test {
result: local;
}
}
9 changes: 0 additions & 9 deletions test/less/import-plugin-scoped.less

This file was deleted.

5 changes: 0 additions & 5 deletions test/less/import-plugin-tiered.less

This file was deleted.

5 changes: 0 additions & 5 deletions test/less/import-plugin.less

This file was deleted.

85 changes: 85 additions & 0 deletions test/less/plugin.less
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// importing plugin globally
@plugin "./plugin/plugin-global";

// transitively include plugins from importing another sheet
@import "./plugin/plugin-transitive";


// `test-global` function should be reachable
// `test-local` function should not be reachable
// `test-shadow` function should return global version
.class {
trans : test-transitive();
global : test-global();
local : test-local();
shadow : test-shadow();

// `test-global` function should propagate and be reachable
// `test-local` function should be reachable
// `test-shadow` function should return local version, shadowing global version
.local {
@plugin "./plugin/plugin-local";
global : test-global();
local : test-local();
shadow : test-shadow();
}
}

// calling a mixin or detached ruleset should not bubble local plugins
// imported inside either into the parent scope.
.mixin() {
@plugin "./plugin/plugin-local";
mixin-local : test-local();
mixin-global : test-global();
mixin-shadow : test-shadow();
}
@ruleset : {
@plugin "./plugin/plugin-local";
ruleset-local : test-local();
ruleset-global : test-global();
ruleset-shadow : test-shadow();
};
#ns {
@plugin "./plugin/plugin-local";
.mixin() {
ns-mixin-global : test-global();
ns-mixin-local : test-local();
ns-mixin-shadow : test-shadow();
}
}
.class {
#ns > .mixin();
.mixin();
@ruleset();
class-local : test-local();
}


// `test-global` function should propagate into directive scope
@media screen {
.test {
result : test-global();
}
}
@font-face {
result : test-global();
}

// `test-global` function should propagate into nested directive scopes
@media screen and (min-width:100px) {
@media (max-width:400px) {
.test {
result : test-global();
}
}
}

.test {
@media screen {
@plugin "./plugin/plugin-local";
result : test-local();
}
}



9 changes: 9 additions & 0 deletions test/less/plugin/plugin-global.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@

functions.addMultiple({
"test-shadow" : function() {
return new tree.Anonymous( "global" );
},
"test-global" : function() {
return new tree.Anonymous( "global" );
}
});
8 changes: 8 additions & 0 deletions test/less/plugin/plugin-local.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
functions.addMultiple({
"test-shadow" : function() {
return new tree.Anonymous( "local" );
},
"test-local" : function() {
return new tree.Anonymous( "local" );
}
});
5 changes: 5 additions & 0 deletions test/less/plugin/plugin-transitive.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
functions.addMultiple({
"test-transitive" : function() {
return new tree.Anonymous( "transitive" );
}
});
5 changes: 5 additions & 0 deletions test/less/plugin/plugin-transitive.less
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@plugin "plugin-transitive";

.other {
trans : test-transitive();
}
4 changes: 0 additions & 4 deletions test/less/plugins/test.js

This file was deleted.

0 comments on commit 86fa4f6

Please sign in to comment.