fix(material/schematics): don't insert duplicate @use statements in themingApi
#22755
Conversation
jelbourn
added
P2
google-cla
bot
added
the
cla: yes
| @@ -205,6 +205,11 @@ function renameSymbols(content: string, | |||
| /** Inserts an `@use` statement in a string. */ | |||
| function insertUseStatement(content: string, importPath: string, importsToIgnore: string[], | |||
| namespace: string): string { | |||
| // If the content already has the `@use` import, we don't need to add anything. | |||
| if (content.includes(`@use '${importPath}'`)) { | |||
There was a problem hiding this comment.
- This doesn't account for imports using double quotes. There's a regex in
detectImportsthat we can modify and reuse here. - I think that we'll have to account for the
namespaceparameter.
There was a problem hiding this comment.
Changed to use the regex from detectImports to account for both quotes. I don't quite see, though, why we'd need to check the namespace here. My thinking was that if there's an import for the importPath at all, it counts as already imported. I suppose that the file could be using a different namespace/alias, but I'm not particularly worried about supporting that case.
| `$another: $mat-pink;`, | ||
| ].join('\n'); | ||
|
|
||
| const migratedContent = migrateFileContent( |
There was a problem hiding this comment.
This should invoke the migration through the schematics API like the other tests, rather than calling migrateFileContent.
jelbourn
force-pushed
the
sass-api-no-dupe-use
branch
from
cfc90e0
to
e89441b
Compare
| @@ -207,6 +207,12 @@ function renameSymbols(content: string, | |||
| /** Inserts an `@use` statement in a string. */ | |||
| function insertUseStatement(content: string, importPath: string, importsToIgnore: string[], | |||
| namespace: string): string { | |||
| // If the content already has the `@use` import, we don't need to add anything. | |||
| const alreadyImportedPattern = new RegExp(`@(import|use) +['"]${importPath}['"]`, 'g'); | |||
There was a problem hiding this comment.
I think that this should be just @use not @(import|use).
… themingApi In the case where a file has mixed legacy and new theming API usage, the script would add an extra duplicate `@use` statement.
jelbourn
force-pushed
the
sass-api-no-dupe-use
branch
from
e89441b
to
87a3b76
Compare
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
In the case where a file has mixed legacy and new theming API usage, the script would
add an extra duplicate
@usestatement.