-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Conversation
Fixed scope for plugins to apply correctly for mixins, directives and detached rulesets. Updated plugin unit tests to be more comprehensive
functionRegistry was mistakingly inheriting from the top frame of the caller context, which was incorrect. It should inherit from definition scope.
@@ -0,0 +1,11 @@ | |||
#ns { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was this file checked in by mistake?
Oops. Yes. I thought I flagged that one as 'do not commit'. Maybe the GitHub client glitched. |
There we go. Deleted and ammended. |
@@ -144,7 +145,16 @@ ImportVisitor.prototype = { | |||
} | |||
}, | |||
visitRule: function (ruleNode, visitArgs) { | |||
visitArgs.visitDeeper = false; | |||
if (ruleNode.value.type === "DetachedRuleset") { |
There was a problem hiding this comment.
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 pull request fixes remaining scoping issues with the
@plugin
mechanic that #2479 introduced.@plugin
inside a directive block is now in scope for descendants of the directive block.@plugin
to cover scope inheritance and shadowing cases.