Skip to content

Commit c2f4ecf

Browse files
jathakjathak
committed
Support import-only files with include rules
This enables migration of some nested/late imports that the migrator couldn't previously handle by first wrapping the file that's depended on via nested import in a mixin and then `@include`ing that mixin in the import-only file.
1 parent 6624ad6 commit c2f4ecf

9 files changed

Lines changed: 286 additions & 15 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,56 @@
1+
## 2.6.0
2+
3+
### Module Migrator
4+
5+
* Add support for import-only files that include a mixin from their
6+
corresponding regular file. For example, given code like:
7+
8+
`styles.scss`:
9+
```scss
10+
a {
11+
$color: blue;
12+
@import "library";
13+
background: $bg;
14+
}
15+
```
16+
17+
`_library.scss`:
18+
```scss
19+
$bg: gold;
20+
@mixin styles($color) {
21+
b {
22+
color: $color;
23+
}
24+
}
25+
```
26+
27+
`_library.import.scss`:
28+
```
29+
@forward "library";
30+
@use "library";
31+
@include library.styles($color);
32+
```
33+
34+
the migrator will migrate `styles.scss` to:
35+
```
36+
@use "library";
37+
38+
a {
39+
$color: blue;
40+
@include library.styles($color);
41+
background: library.$bg;
42+
}
43+
```
44+
45+
While the migrator won't generate import-only files with this structure, this
46+
support for them will enable migration of projects that use nested and late
47+
imports that both emit CSS and provide Sass members by first manually
48+
wrapping the nested imports' CSS in mixins and then running the migrator to
49+
actually handle the nested imports.
50+
51+
Arguments can be passed to the mixin included here, but each argument must
52+
consist of a single variable or the migrator will error.
53+
154
## 2.5.7
255

356
* Fix a bug where the `--migrate-deps` flag would not apply to dependencies

‎lib/src/migrators/module.dart‎

Lines changed: 69 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,12 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
172172
assertInStylesheet(__hoistedUseRules, '_hoistedUseRules');
173173
Set<String>? __hoistedUseRules;
174174

175+
/// Set of `@include` rules from import-only files that need to be added after
176+
/// all `@use` and `@forward` rules.
177+
Set<String> get _postUseIncludeRules =>
178+
assertInStylesheet(__postUseIncludeRules, '_postUseIncludeRules');
179+
Set<String>? __postUseIncludeRules;
180+
175181
/// Set of additional `@use` rules for stylesheets at a load path.
176182
Set<String> get _additionalLoadPathUseRules => assertInStylesheet(
177183
__additionalLoadPathUseRules, '_additionalLoadPathUseRules');
@@ -457,6 +463,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
457463
var oldUsedUrls = __usedUrls;
458464
var oldBuiltInUseRules = __builtInUseRules;
459465
var oldHoistedUseRules = __hoistedUseRules;
466+
var oldPostUseIncludeRules = __postUseIncludeRules;
460467
var oldLoadPathUseRules = __additionalLoadPathUseRules;
461468
var oldRelativeUseRules = __additionalRelativeUseRules;
462469
var oldBeforeFirstImport = _beforeFirstImport;
@@ -475,6 +482,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
475482
__forwardedUrls = {};
476483
__builtInUseRules = {};
477484
__hoistedUseRules = {};
485+
__postUseIncludeRules = {};
478486
__additionalLoadPathUseRules = {};
479487
__additionalRelativeUseRules = {};
480488
_beforeFirstImport = null;
@@ -486,6 +494,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
486494
__usedUrls = oldUsedUrls;
487495
__builtInUseRules = oldBuiltInUseRules;
488496
__hoistedUseRules = oldHoistedUseRules;
497+
__postUseIncludeRules = oldPostUseIncludeRules;
489498
__additionalLoadPathUseRules = oldLoadPathUseRules;
490499
__additionalRelativeUseRules = oldRelativeUseRules;
491500
_beforeFirstImport = oldBeforeFirstImport;
@@ -496,22 +505,23 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
496505
/// Adds additional patches for extra `@use` and `@forward` rules.
497506
@override
498507
void beforePatch(Stylesheet node) {
499-
useRulesToString(Set<String> useRules) =>
500-
(useRules.toList()..sort()).map((use) => '$use$semicolon\n').join();
508+
rulesToString(Set<String> rules) =>
509+
(rules.toList()..sort()).map((rule) => '$rule$semicolon\n').join();
501510

502511
if (_builtInUseRules.isNotEmpty) {
503512
// This is added before existing patches to ensure that this patch is
504513
// inserted before a patch converting an existing `@import` rule to a
505514
// `@use` rule.
506515
addPatch(
507516
Patch.insert(_beforeFirstImport ?? node.span.start,
508-
useRulesToString(_builtInUseRules)),
517+
rulesToString(_builtInUseRules)),
509518
beforeExisting: true);
510519
}
511-
var extras = useRulesToString(_hoistedUseRules) +
512-
useRulesToString(_additionalLoadPathUseRules) +
513-
useRulesToString(_additionalRelativeUseRules) +
514-
_getAdditionalForwardRules();
520+
var extras = rulesToString(_hoistedUseRules) +
521+
rulesToString(_additionalLoadPathUseRules) +
522+
rulesToString(_additionalRelativeUseRules) +
523+
_getAdditionalForwardRules() +
524+
rulesToString(_postUseIncludeRules);
515525
if (extras == '') return;
516526
var insertionPoint = _afterLastImport ?? node.span.start;
517527
// If the insertion point is in the middle of a line, add a line break
@@ -831,7 +841,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
831841
String rulesText;
832842

833843
var inPlaceUseRules = <String>[];
834-
var loadCssRules = <String>[];
844+
var includeRules = <String>[];
835845

836846
var indent = ' ' * node.span.start.column;
837847

@@ -855,6 +865,11 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
855865
var canonicalUrl = importCache
856866
.canonicalize(ruleUrl, baseImporter: importer, baseUrl: currentUrl)!
857867
.$2;
868+
var canonicalImport = importCache
869+
.canonicalize(ruleUrl,
870+
baseImporter: importer, baseUrl: currentUrl, forImport: true)!
871+
.$2;
872+
var importOnlyInclude = references.importOnlyIncludes[canonicalImport];
858873
var isNested = !_currentStylesheet.children.contains(node);
859874
if (builtInOnly) {
860875
if (migrateDependencies) {
@@ -864,15 +879,23 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
864879
}
865880
} else if (_useAllowed.canMigrateInPlace) {
866881
inPlaceUseRules.addAll(_migrateImportToRules(ruleUrl, import.span));
867-
} else if (!isNested &&
882+
if (importOnlyInclude != null) {
883+
_postUseIncludeRules
884+
.add(_makeIncludeFromImportOnly(import, importOnlyInclude));
885+
}
886+
} else if ((!isNested || importOnlyInclude != null) &&
868887
(_useAllowed.canAlwaysSafelyHoist ||
869888
!(references.fileEmitsCss[canonicalUrl] ?? true) ||
870889
(unsafeHoist &&
871890
references.anyMemberReferenced(
872891
canonicalUrl, currentUrl)))) {
873892
_hoistedUseRules.addAll(_migrateImportToRules(ruleUrl, import.span));
893+
if (importOnlyInclude != null) {
894+
includeRules
895+
.add(_makeIncludeFromImportOnly(import, importOnlyInclude));
896+
}
874897
} else {
875-
loadCssRules.add(
898+
includeRules.add(
876899
_migrateImportToLoadCss(ruleUrl, import.span, isNested)
877900
.replaceAll('\n', '\n$indent'));
878901
}
@@ -881,7 +904,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
881904
if (builtInOnly) return;
882905

883906
rulesText =
884-
[...inPlaceUseRules, ...loadCssRules].join('$semicolon\n$indent');
907+
[...inPlaceUseRules, ...includeRules].join('$semicolon\n$indent');
885908
if (rulesText.isEmpty) {
886909
var span = node.span.extendIfMatches(RegExp(' *$semicolon\n?'));
887910
addPatch(patchDelete(span));
@@ -990,6 +1013,41 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
9901013
return '@include $meta.load-css($quotedUrl$configuration)';
9911014
}
9921015

1016+
/// Creates an `@include` rule for the current file based on the `@include`
1017+
/// rule in an import-only file, adding any additional `@use` rules as
1018+
/// necessary.
1019+
String _makeIncludeFromImportOnly(DynamicImport import, IncludeRule rule) {
1020+
var declaration = references.mixins[rule];
1021+
if (declaration == null) {
1022+
throw MigrationSourceSpanException(
1023+
"Couldn't find mixin referenced by import-only file.", import.span);
1024+
}
1025+
var namespace = _namespaceForDeclaration(declaration);
1026+
var args = [
1027+
for (var arg in rule.arguments.positional)
1028+
switch (arg) {
1029+
VariableExpression(:var name, namespace: _?) =>
1030+
'${_namespaceForDeclaration(references.variables[arg]!)}.\$$name',
1031+
VariableExpression(:var name) => '\$$name',
1032+
_ => throw MigrationSourceSpanException(
1033+
'Import-only @include rules may only pass variables as arguments.',
1034+
arg.span),
1035+
},
1036+
for (var MapEntry(key: parameter, value: arg)
1037+
in rule.arguments.named.entries)
1038+
switch (arg) {
1039+
VariableExpression(:var name, namespace: _?) => '\$$parameter: '
1040+
'${_namespaceForDeclaration(references.variables[arg]!)}.\$$name',
1041+
VariableExpression(:var name) => '\$$parameter: \$$name',
1042+
_ => throw MigrationSourceSpanException(
1043+
'Import-only @include rules may only pass variables as arguments.',
1044+
arg.span),
1045+
}
1046+
];
1047+
var argString = args.isEmpty ? '' : '(${args.join(', ')})';
1048+
return '@include $namespace.${rule.name}$argString';
1049+
}
1050+
9931051
/// Common logic for migrating imports shared by both the normal migration to
9941052
/// `@use`/`@forward` rules and the migration to `meta.load-css`.
9951053
///

‎lib/src/migrators/module/references.dart‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,10 @@ class References {
9292
/// file that itself emits CSS.
9393
final Map<Uri, bool> fileEmitsCss;
9494

95+
/// A map from import-only files to top-level `@include` rules that should be
96+
/// used when migrating that import.
97+
final Map<Uri, IncludeRule> importOnlyIncludes;
98+
9599
/// An iterable of all member declarations.
96100
Iterable<MemberDeclaration> get allDeclarations =>
97101
variables.values.followedBy(mixins.values).followedBy(functions.values);
@@ -150,7 +154,8 @@ class References {
150154
Map<MemberDeclaration, Set<Uri>> libraries,
151155
Map<SassReference, ReferenceSource> sources,
152156
Map<Uri, ForwardRule?> orphanImportOnlyFiles,
153-
Map<Uri, bool> fileEmitsCss)
157+
Map<Uri, bool> fileEmitsCss,
158+
Map<Uri, IncludeRule> importOnlyIncludes)
154159
: variables = UnmodifiableBidirectionalMapView(variables),
155160
variableReassignments =
156161
UnmodifiableBidirectionalMapView(variableReassignments),
@@ -167,7 +172,8 @@ class References {
167172
}),
168173
sources = UnmodifiableMapView(sources),
169174
orphanImportOnlyFiles = UnmodifiableMapView(orphanImportOnlyFiles),
170-
fileEmitsCss = UnmodifiableMapView(fileEmitsCss);
175+
fileEmitsCss = UnmodifiableMapView(fileEmitsCss),
176+
importOnlyIncludes = UnmodifiableMapView(importOnlyIncludes);
171177

172178
/// Constructs a new [References] object based on a [stylesheet] (imported by
173179
/// [importer]) and its dependencies.
@@ -195,6 +201,7 @@ class _ReferenceVisitor extends ScopedAstVisitor {
195201
final _sources = <SassReference, ReferenceSource>{};
196202
final _orphanImportOnlyFiles = <Uri, ForwardRule?>{};
197203
final _fileEmitsCss = <Uri, bool>{};
204+
final _importOnlyIncludes = <Uri, IncludeRule>{};
198205

199206
/// Mapping from canonical stylesheet URLs to the global scope of the module
200207
/// contained within it.
@@ -296,7 +303,8 @@ class _ReferenceVisitor extends ScopedAstVisitor {
296303
_libraries,
297304
_sources,
298305
_orphanImportOnlyFiles,
299-
_fileEmitsCss);
306+
_fileEmitsCss,
307+
_importOnlyIncludes);
300308
}
301309

302310
/// Checks any remaining [_unresolvedReferences] to see if they match a
@@ -701,6 +709,16 @@ class _ReferenceVisitor extends ScopedAstVisitor {
701709
} else if (namespace == null) {
702710
_unresolvedReferences[node] = currentScope;
703711
}
712+
if (isImportOnlyFile(_currentUrl)) {
713+
if (_importOnlyIncludes.containsKey(_currentUrl)) {
714+
throw MigrationSourceSpanException(
715+
"Found a second @include rule in an import-only file. "
716+
"Import-only files should contain at most one @include.",
717+
node.span);
718+
}
719+
_importOnlyIncludes[_currentUrl] = node;
720+
_isOrphanImportOnly = false;
721+
}
704722
}
705723

706724
/// Declares a function in the current scope.

‎pubspec.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: sass_migrator
2-
version: 2.5.7
2+
version: 2.6.0
33
description: A tool for running migrations on Sass files
44
homepage: https://github.com/sass/migrator
55

‎test/migrators/module/partial_migration/import_only_include/ensure_after_other_use_rules.hrx‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<==> arguments
2+
--migrate-deps
3+
4+
<==> input/entrypoint.scss
5+
@import "library";
6+
@import "library2";
7+
8+
<==> input/_library.scss
9+
@mixin styles {
10+
a {
11+
b: c;
12+
}
13+
}
14+
15+
<==> input/_library2.scss
16+
// Ignore this
17+
18+
<==> input/_library.import.scss
19+
@use "library";
20+
@include library.styles;
21+
22+
<==> output/entrypoint.scss
23+
@use "library";
24+
@use "library2";
25+
@include library.styles;

‎test/migrators/module/partial_migration/import_only_include/no_arguments_no_forward.hrx‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<==> arguments
2+
--migrate-deps
3+
4+
<==> input/entrypoint.scss
5+
a {
6+
@import "library";
7+
}
8+
9+
<==> input/_library.scss
10+
@mixin styles {
11+
x {
12+
y: z;
13+
}
14+
}
15+
16+
<==> input/_library.import.scss
17+
@use "library";
18+
@include library.styles;
19+
20+
<==> output/entrypoint.scss
21+
@use "library";
22+
23+
a {
24+
@include library.styles;
25+
}

‎test/migrators/module/partial_migration/import_only_include/with_argument.hrx‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<==> arguments
2+
--migrate-deps
3+
4+
<==> input/entrypoint.scss
5+
$var: green;
6+
7+
a {
8+
@import "library";
9+
}
10+
11+
<==> input/_library.scss
12+
@mixin styles($var) {
13+
x {
14+
b: $var;
15+
}
16+
}
17+
18+
<==> input/_library.import.scss
19+
@use "library";
20+
@include library.styles($var);
21+
22+
<==> output/entrypoint.scss
23+
@use "library";
24+
25+
$var: green;
26+
27+
a {
28+
@include library.styles($var);
29+
}

0 commit comments

Comments
 (0)