-
-
Notifications
You must be signed in to change notification settings - Fork 78.8k
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: variables collide with globals #32492
Conversation
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.
Looks good to me; @twbs/css-review any thoughts? Should this be documented somewhere?
Does this count as a breaking change? |
This is an internal loop, declaring a mixin: I don't see what could break since |
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.
Wow, I didn't know global variables were overwritten within these loops, but I can confirm this is the case:
https://www.sassmeister.com/gist/fc876c989dbc0920e2d4373aa262ec85?token=203daf3221af119f9e7a6898b97e14608c077727&scope=gist,read:user
I double checked if the $state
& $value
variables were overridden, but it turns out that's a new scope, meaning we don't have to worry about
Lines 58 to 68 in f12657b
@each $color, $value in $theme-colors { | |
.btn-#{$color} { | |
@include button-variant($value, $value); | |
} | |
} | |
@each $color, $value in $theme-colors { | |
.btn-outline-#{$color} { | |
@include button-outline-variant($value); | |
} | |
} |
It's not a BC and there's no reason to document this, since these are variables set in a loop. We could move the alert variables inside the selector to prevent them becoming global. But for the list groups, we still need global variables, so prefixing is fine!
Variables($background, $border, $color) in
@each
loop pollutes global scopes.And don't let define new variables with these names in custom SCSS file.