Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix @plugin scoping rules #2522

Merged
merged 6 commits into from
Apr 1, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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") {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import and plugin directives inside detached rulesets didn't work as expected because the visitor was stopping at the rule level, never entering the detached ruleset declaration.

This change enables it for the simple form of @var : { ... } and verifies that scoping for plugin works as expected in that scenario. You may want to not take this particular change. Just notifying and trying to be clear.

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.