Currently, if a file imported through node_modules contains a relative import, that relative URL is used as-is in referencing files:

<==> input/entrypoint.scss
@import "~module/dependency";

a {
  color: $variable;
}

<==> input/node_modules/module/_dependency.scss
@import "other";

<==> input/node_modules/module/_other.scss
$variable: green;

This should produce @use "~module/other", but it currently produces @use "other" which is just broken.

This is a meta-issue to track all the sub-components of the complete implementation of the /-as-division migrator.

• Support multiple migrators
• Initial division migrator
• Treat / as division in argument lists
• Only generate slash-list() outside plain CSS contexts

When a path to a file that doesn't exist is passed to the migrator, it crashes:

$ dart bin/sass_migrator.dart division asdf
Unhandled exception:
NoSuchMethodError: The getter 'scheme' was called on null.
Receiver: null
Tried calling: scheme
#0      Object.noSuchMethod (dart:core-patch/object_patch.dart:50:5)
#1      FilesystemImporter.canonicalize (package:sass/src/importer/filesystem.dart:24:13)
#2      parseStylesheet (package:sass_migrator/src/utils.dart:29:42)
#3      MigrationVisitor.run (package:sass_migrator/src/migration_visitor.dart:46:21)
#4      DivisionMigrator.migrateFile (package:sass_migrator/src/migrators/division.dart:38:12)
#5      Migrator.run (package:sass_migrator/src/migrator.dart:48:22)
#6      CommandRunner.runCommand (package:args/command_runner.dart:196:27)
<asynchronous suspension>
#7      MigratorRunner.execute (package:sass_migrator/runner.dart:45:26)
<asynchronous suspension>
#8      main (file:///home/nweiz/sass/migrator/bin/sass_migrator.dart:16:20)
#9      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:298:32)
#10     _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:171:12)

Actually, looking at these examples, they're really hard on the eyes. After this lands, can we make the migrator smart enough to only migrate to slash-list() in a non-plain-CSS context? Basically, we should generate slash-list() only if either _isDivisionAllowed is true or one of the arguments (possibly deeply-nested) is an interpolation expression.

Originally posted by @nex3 in #17

Re: #13 (comment)

This will require changing the tests to check stderr instead of printed output, which will probably require using test_process.

The indented syntax allows a single @import rule to import multiple stylesheets at once. The migrator currently assumes that each @import loads one file, but it should support @imports that load multiple files at once.

#14 and to some extent #24 are blocked on being able to capture stderr when testing. Doing that will require using test_process instead of calling the migrators directly.

Fixing this should also allow canonicalize and parseStylesheet to use a single FilesystemImporter instance instead of a new one each time (right now, doing this would interfere with out ability to change the directory the migrator is running from in tests).

Something like this

// entrypoint.scss
$config: red;
@import "direct";

// _direct.scss
@import "indirect";

// _indirect.scss
$config: green !default;

is currently migrated to

// entrypoint.scss
$config: red;
@use "direct";

// _direct.scss
@use "entrypoint";
@use  "indirect";

// _indirect.scss
$config: green !default;

which is definitely wrong, but I'm also not sure what the best way to actually do it would be.

I just realized, this should also be set to true in argument lists for functions and mixins, except for the specific case of new-syntax rgb() et al functions. Feel free to fix after landing this PR, though.

Originally posted by @nex3 in #17

The module migrator currently creates @use rules in the order that their members are used, which ends up looking kind of messy. It would be good at some point to produce a more principled ordering. I propose:

• sass: @uses, in alphabetical order.
• Any @uses that come from load paths, in alphabetical order.
• Relative @uses, in alphabetical order.
• A newline.
• Same order for @forwards as above.
• A newline if there were any @forwards.

Heads up: for historical reasons, variables, functions, and mixins in Sass treat underscores and hyphens as identical, so a literal text comparison isn't sufficient in the general case. Feel free to crib Dart Sass's normalizedMap() and normalizedSet() functions to handle this.

Originally posted by @nex3 in https://github.com/sass/module_migrator/diffs

If you pass an unknown command to the migrator, it fails gracelessly:

$ dart bin/sass_migrator.dart asdf
Unhandled exception:
Could not find a command named "asdf".

Usage: sass_migrator <migrator> [options] <entrypoint.scss...>

Global options:
-h, --help                 Print this usage information.
-d, --[no-]migrate-deps    Migrate dependencies in addition to entrypoints.
-n, --[no-]dry-run         Show which files would be migrated but make no changes.
    --[no-]unicode         Whether to use Unicode characters for messages.
-v, --[no-]verbose         Print text of migrated files when running with --dry-run.

Available commands:
  division   Migrates from the / division operator to the divide function
  help       Display help information for sass_migrator.
  module     Migrates stylesheets to the new module system.

Run "sass_migrator help <command>" for more information about a command.
#0      CommandRunner.usageException (package:args/command_runner.dart:94:7)
#1      CommandRunner.runCommand (package:args/command_runner.dart:162:13)
<asynchronous suspension>
#2      MigratorRunner.execute (package:sass_migrator/runner.dart:45:26)
<asynchronous suspension>
#3      main (file:///usr/local/google/home/nweiz/goog/sass/migrator/bin/sass_migrator.dart:16:20)
#4      _startIsolate.<anonymous closure> (dart:isolate/runtime/libisolate_patch.dart:298:32)
#5      _RawReceivePortImpl._handleMessage (dart:isolate/runtime/libisolate_patch.dart:171:12)

(Part of #15)

If a dependency is already migrated, the migrator should still be able to migrate the stylesheet depending on it, without needing to re-migrate the dependency.

This will also include unprefixing members when there is a <library>.import.scss that forwards members with that prefix.

A few function overloads exist to provide compatibility with plain CSS function calls, and should not be migrated to the new module system. These are:

• grayscale() with a number argument.
• invert() with a number argument.
• opacity() with a number argument.
• alpha() with multiple arguments, or with a single argument that's a single-equals expression.
• saturate() with a single argument.

See also sass/sass#2703.

To allow the migrator to be used for stylesheets that use the custom webpack importer that resolves imports like

@import "~package/path/file"

to node_modules/package/path/file.scss, add support for this syntax

When a @forward is added for a given stylesheet, that stylesheet should omit the @use rule for the same stylesheet unless its members are actually being used.

Right now, if a dependency is migrated in multiple ways by multiple entrypoints, the migrator will error with a message that's not particularly helpful.

This should instead display the diff between the two migrations, and ideally give the user a choice of which one to use if the migrator is running in a terminal.

Currently (on both master and partial-migration), the migrator generates invalid Sass for the following stylesheets:

// style.scss
@mixin foo { /* ... */ }
@import "other";

// _other.scss
@include foo;

It converts style.scss to use meta.load-css(), but it doesn't change _other.scss at all. This isn't a super common case, but it's something that should be handled somehow.

This is a strange case. @include foo references a member defined at the top level of another file, but it can't directly @use that file because doing so would create a circular reference. At the same time, the user probably intended to use style.scss as a kind of library of members usable by everything imported afterwards, which is totally reasonable. The ideal migration would probably be:

// style.scss
@use "other";

// _members.scss
@mixin foo { /* ... */ }

// _other.scss
@use "members";
@include members.foo;

but moving members into a totally new file would probably is likely to add a lot of complexity. So maybe the best thing to do right now is to add a new category of unreferencable member and throw an error encouraging the user to manually move those members into their own file.

Right now something like:

// entrypoint.scss
$background: mix(red, blue)
@import "library";

// _library.scss
$background: green !default;

would error, since the patch to add the color namespace and the patch to delete the configuration variable would conflict.

We should handle this case by removing the existing patches and instead applying them to the configuration, e.g:

@use "sass:color";
@use "library" with ($background: color.mix(red, blue));

If a Sass file contains a parse error, the migrator will currently print a stack trace rather than gracefully handling the error.

This has two parts:

• Unprefix members when migrating a library to the module system (#21)
• Migrate stylesheets with dependencies that are already migrated, including unprefixing if necessary (#22)

Original issue:

Right now, the migrator pretty much assumes that the entrypoint(s) and all dependencies will be migrated together, which only really works when all stylesheets are local to one project. The migrator should have a mode designed for migrating libraries without breaking anything downstream.

This should support the following:

• When migrating library.scss, the migrator should also create a library.import.scss file that maintains the pre-migration behavior but warns when imported.
• If a library previously explicitly namespaced its members with a prefix (e.g. lib-foo), the migrator should be able to automatically remove the prefixes and instead rely on the module namespace (e.g. library.foo)
• When migrating a stylesheet that depends on a previously migrated library, the migrator should be able to remove those prefixes, ideally without the user having to explicitly specify the prefixes to be removed.

If you remove a prefix without migrating dependencies, and the files you pass on the command line include the use of a prefixed member but not that member's definition, the uses will be migrated but the definition will not rendering those uses invalid. For example:

// input.scss
@import "a";
@debug $a-b;

// _a.scss
$a-b: c;

Running sass-migrator module --remove-prefix=a- input.scss will result in

// input.scss
@use "a";
@debug a.$b`

// _a.scss
$a-b: c;

It's probably best to just throw an error here and encourage users to re-run with either --migrate-deps or with _a.scss passed on the command line as well.

When the migrator encounters a parse error in an input file, it crashes:

$ echo 'a {' > test.scss
$ dart bin/sass_migrator.dart division test.scss                                                
Unhandled exception:
Error: expected "}".
  ,
1 | a {
  |    ^
  '
  test.scss 1:4  root stylesheet
#0      Parser.wrapSpanFormatException (package:sass/src/parse/parser.dart:663:7)
#1      StylesheetParser.parse (package:sass/src/parse/stylesheet.dart:93:12)
#2      new Stylesheet.parseScss (package:sass/src/ast/sass/statement/stylesheet.dart:61:54)
#3      new Stylesheet.parse (package:sass/src/ast/sass/statement/stylesheet.dart:39:27)
#4      parseStylesheet (package:sass_migrator/src/utils.dart:31:21)
#5      MigrationVisitor.run (package:sass_migrator/src/migration_visitor.dart:46:21)
#6      DivisionMigrator.migrateFile (package:sass_migrator/src/migrators/division.dart:38:12)
#7      Migrator.run (package:sass_migrator/src/migrator.dart:48:22)
#8      CommandRunner.runCommand (package:args/command_runner.dart:196:27)
<asynchronous suspension>
#9      MigratorRunner.execute (package:sass_migrator/src/runner.dart:51:26)
<asynchronous suspension>
#10     main (file:///home/nweiz/sass/migrator/bin/sass_migrator.dart:16:20)
#11     _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:298:32)
#12     _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:171:12)

Some libraries already prefix members with _ or - to indicate that they're library-private. If any of these members are used across modules, though, leaving their names as-is will break, since they're now treated as module-private. The migrator should rename such members to remove the leading _ or -.

Once #47 is done, the migrator should also guarantee that members that used to start with _ or - are never forwarded through top-level entrypoints (using hide if necessary).

For projects with many entrypoints, it would be useful to be able to pass in a glob of files to migrate (such as sass-migrator division **/*.scss).

If you remove a prefix that's followed by a - in a member name, the migrator won't detect the new member as private and will accordingly generate invalid member references. For example, with --remove-prefix=a, $a-b will become module.$-b, which is invalid.

This seems like a likely thing for users to do by accident if they don't realize that they're supposed to include the trailing - in the prefix.

See #63 (comment)

Low priority for now, until more migrators that use missingDependencies exist.

With @import, it's possible for a file to refer to a mixin or function that gets redefined later on. Right now References will just override the previous definition and treat it as though it always referred to whatever the last value was, but the more correct behavior would be to throw an error, since changing the definition isn't supported in the module system.

For example:

<==> input/entrypoint.scss
@import "definition1";
@import "upstream";
@import "midstream";

<==> input/definition1.scss
@function fn() {@return 1}

<==> input/definition2.scss
@function fn() {@return 2}

<==> input/midstream.scss
@import "definition2";
@import "upstream";

<==> input/upstream.scss
a {b: fn()}

I don't think this is worth fixing before we launch, though.

When running in --migrate-deps mode, the migrator currently only ever resolves imports relative to the file that contains them. In real-world stylesheets, though, imports often come through load paths and custom importers. We need to handle these well.

For the division migrator, we can probably get away with just not traversing imports that can't be resolved, since we don't need contextual information from them to do the migration. For the module migration, it's harder; we need to know what names each import defines to be able to translate them into module references. Load paths can eventually be passed to the executable, but importers pose a bigger problem. Can we somehow have the migrator load them from JS?

See also #57

$namespace.variable -> namespace.$variable

Can you extend RecursiveStatementVisitor to handle the nitty-gritty of tree traversal?

Originally posted by @nex3 in https://github.com/sass/module_migrator/diffs

When attempting to a remove a prefix from a stylesheet like

$lib-var: red;

a {
    $lib-var: blue !global;
}

the migrator crashes, since it attempts to patch the name of the second $lib-var declaration twice.

I believe this is due to the parser adding extra VariableDeclarations to the top level of the Stylesheet to work around sass/sass#2647.

We need to detect these fake declarations and avoid visiting them (or at least avoid patching them) in the module migrator.

Existing uses of / as a division operator should be migrated to the use the new divide function (or math.div in the new module system)

At some point, it would be good to remove variable declarations that are used to configure an import. Maybe file an issue for that?

Originally posted by @nex3 in #4

In order to reach the full Sass audience, this package should eventually be distributed across all the same channels as Dart Sass:

• npm (blocked on #25)
• GitHub releases
• pub
• Homebrew
• Chocolatey

(Part of #15)

The user should be able to specify a prefix to remove from all members when migrating a library to the new module system. The migrator should also generate a <library>.import.scss file that forwards all members with the prefix to allow for migrating a library without breaking downstream dependencies.

It's unlikely that users will want dependencies loaded through their load path or node_modules to get migrated even with --migrate-deps, since those are generally not authored by the same people as the main application.

See sass/sass#77

For those whose referent can't be statically determined, print warnings.

This package will eventually need to be distributed to our many Node.js users, so it should support running on Node.js as well as the Dart VM.

Nested @imports should be replaced with @include meta.load-css().

    print('Migrates one or more Sass files to the new module system.\n\n'

This should ultimately work with the indented syntax as well as SCSS.

Originally posted by @nex3 in https://github.com/sass/module_migrator/pull/4/files

When running the migrator on a library, authors will usually want to expose all the members that library defines from the library's entrypoint file (that is, the file passed initially to the migrator). The migrator should support a flag to do this automatically by @forwarding all the files that define public names that are transitively imported by the migrator.

When the migrator encounters a URL that can't be loaded in --migrate-deps mode, it crashes:

$ echo "@import 'asdf';" > test.scss
$ dart bin/sass_migrator.dart division -d test.scss
Unhandled exception:
Invalid argument (uri): Value must be a String or a Uri: null
#0      _parseUri (package:path/src/context.dart:1065:3)
#1      Context.fromUri (package:path/src/context.dart:986:44)
#2      fromUri (package:path/path.dart:410:32)
#3      FilesystemImporter.load (package:sass/src/importer/filesystem.dart:30:16)
#4      parseStylesheet (package:sass_migrator/src/utils.dart:30:36)
#5      MigrationVisitor.visitDependency (package:sass_migrator/src/migration_visitor.dart:71:22)
#6      MigrationVisitor.visitImportRule (package:sass_migrator/src/migration_visitor.dart:99:11)
#7      ImportRule.accept (package:sass/src/ast/sass/statement/import_rule.dart:21:55)
#8      RecursiveStatementVisitor.visitChildren (package:sass/src/visitor/recursive_statement.dart:215:13)
#9      RecursiveStatementVisitor.visitStylesheet (package:sass/src/visitor/recursive_statement.dart:133:41)
#10     MigrationVisitor.visitStylesheet (package:sass_migrator/src/migration_visitor.dart:60:11)
#11     MigrationVisitor.run (package:sass_migrator/src/migration_visitor.dart:46:5)
#12     DivisionMigrator.migrateFile (package:sass_migrator/src/migrators/division.dart:38:12)
#13     Migrator.run (package:sass_migrator/src/migrator.dart:48:22)
#14     CommandRunner.runCommand (package:args/command_runner.dart:196:27)
<asynchronous suspension>
#15     MigratorRunner.execute (package:sass_migrator/runner.dart:45:26)
<asynchronous suspension>
#16     main (file:///home/nweiz/sass/migrator/bin/sass_migrator.dart:16:20)
#17     _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:298:32)
#18     _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:171:12)

#77 did this for URLs loaded with NodeModulesImporter, but we should do the same for load paths

SourceSpans have the ability to colorize their string representations. The migrator should take advantage of this by printing these spans in color, according to the same logic Dart Sass uses.

The README is out-of-date; it still describes this as only the module system migrator.

This is low-priority until we actually want to start distributing this package, but the README should eventually also contain instructions for installing the migrator on various platforms.