sass / migrator

Tool for migrating stylesheets to new Sass versions
MIT License
84 stars 10 forks source link

Module Migration Does Not Handle Negating Imported Variables #114

Closed justinryder closed 4 years ago

justinryder commented 4 years ago

Reduced Test Case

Given the following files:

_variables.scss:

$foo: 1;

styles.scss:

@import 'variables';

.foo {
    padding: -$foo;
}

Run the following command (sass-migrator@1.0.0): sass-migrator module styles.scss

Actual Results

The resulting styles.scss throws a compilation error where the module is used.

@use 'variables';

.foo {
    padding: -variables.$foo;
}

Compilation error (sass@1.23.0):

> npx sass styles.scss
Error: There is no module with the namespace "-variables".
  ╷
4 │     padding: -variables.$foo;
  │              ^^^^^^^^^^^^^^^
  ╵
  styles.scss 4:14  root stylesheet

Expected Results

styles.scss should be migrated with no compilation errors.

Possible valid target output (this compiles successfully with sass@1.23.0):

@use 'variables';

.foo {
    padding: -(variables.$foo);
}
jathak commented 4 years ago

@nex3: What would be the better style for migrating these cases? With parentheses around the namespaced variable or with a space after the -?

nex3 commented 4 years ago

We could consider parsing initial hyphens before IDENTIFIER "." as negations—it requires some amount of lookahead in the parser, but that's par for the course in Sass :laughing:. I'll add it to the design meeting agenda.

For now, it's probably best to just generate parentheses here.