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

Leaky Sass variables #156

Open
lolmaus opened this issue Oct 14, 2019 · 3 comments
Open

Leaky Sass variables #156

lolmaus opened this issue Oct 14, 2019 · 3 comments

Comments

@lolmaus
Copy link

lolmaus commented Oct 14, 2019

Problem

Sass variables defined at module root are global. They leak across files, and changing a variable in one module affects other modules (unless the variable is redefined at the top of each module).

This can lead to nasty bugs. Leaky variables are not what you expect from an addon with module in it's name.

Cause

Unfortunately, Sass does not have a !local flag that would make a variable defined at the root of a file to be local to that file.

Proposed solution

I've come up with a Sass hack to resolve this issue.

I wrap all the module's code with @at-root {}.

Such directive is meaningless: without a selector it does nothing. The resulting CSS is identical to what it used to be before wrapping with @at-root. The specificity of selectors is not affected.

But thanks to a pair of curlies, it has a side-effect: all variables defined inside are now local!

Note that you can still define/overwrite global variables using the !global flag.

Example

app/pods/components/foo-bar/styles.scss

Before:

$padding: 20px;

.header: {
  padding: $padding;
}

.footer {
  padding: $padding;
}

When I wrote the code above, I wasn't aware that a global variable $padding was already used throughout the app. I changed it's value globally.

In the best case, I would receive a compile-time error like incompatible units px and rem.

In the worst case, it would compile, and half of the app's components will end up defaced. The exact amount depends on the alphabetic order of components, I guess.

After:

@at-root {
  $padding: 20px;

  .header: {
    padding: $padding;
  }

  .footer {
    padding: $padding;
  }
}

Now the $padding variable is local to this module. Assigning a value to it does not affect other components that use a global variable with the same name.

TL/DR

We have to wrap every our module with @at-root {} by hand.

It would be nice if ember-css-modules did it automatically.

@dfreeman
Copy link
Member

I think this is a great workaround for folks using Sass and running into scoping issues with their variables. However, I find that often people end up using Sass variables with CSS Modules because they want something that's globally available to all their modules. I don't have a strong opinion on that pattern personally, and tend not to use Sass with ECM in my own projects, but I've witnessed it a lot in the wild.

Either way, ECM itself doesn't have anything Sass-specific in it, and I think that's unlikely to change. This seems like a perfect candidate for an ember-css-modules plugin package, though, to allow Sass users to opt in to this behavior if they want it 🙂

@lolmaus
Copy link
Author

lolmaus commented Oct 14, 2019

However, I find that often people end up using Sass variables with CSS Modules because they want something that's globally available to all their modules.

Global CSS classes are also commonly used by people, and the way to access a global class from a module is :global.

It makes perfect sense to access global variables via !global.

Note: you don't even need to use !global when reading from a variable. It's only necessary when writing into a global variable, which is not something that should be ever done from a module.

@dfreeman
Copy link
Member

It makes perfect sense to access global variables via !global.

Per my previous comment, ECM by design doesn't have any special features or knowledge baked in for Sass (or Less, or Stylus, etc etc) 🙂

As I said, this is exactly the kind of thing the ECM plugin system is for, so that users who do want some kind of special handling can build and share it without needing to expand the core beyond its intended scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants