Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,17 @@ interface ExtraSymbols {
* matched, the prefix would be `~@angular/cdk/`.
* @param newMaterialImportPath New import to the Material theming API (e.g. `~@angular/material`).
* @param newCdkImportPath New import to the CDK Sass APIs (e.g. `~@angular/cdk`).
* @param excludedImports Pattern that can be used to exclude imports from being processed.
*/
export function migrateFileContent(content: string,
oldMaterialPrefix: string,
oldCdkPrefix: string,
newMaterialImportPath: string,
newCdkImportPath: string,
extraMaterialSymbols: ExtraSymbols = {}): string {
const materialResults = detectImports(content, oldMaterialPrefix);
const cdkResults = detectImports(content, oldCdkPrefix);
extraMaterialSymbols: ExtraSymbols = {},
excludedImports?: RegExp): string {
const materialResults = detectImports(content, oldMaterialPrefix, excludedImports);
const cdkResults = detectImports(content, oldCdkPrefix, excludedImports);

// Try to migrate the symbols even if there are no imports. This is used
// to cover the case where the Components symbols were used transitively.
Expand Down Expand Up @@ -77,8 +79,10 @@ export function migrateFileContent(content: string,
* Counts the number of imports with a specific prefix and extracts their namespaces.
* @param content File content in which to look for imports.
* @param prefix Prefix that the imports should start with.
* @param excludedImports Pattern that can be used to exclude imports from being processed.
*/
function detectImports(content: string, prefix: string): DetectImportResult {
function detectImports(content: string, prefix: string,
excludedImports?: RegExp): DetectImportResult {
if (prefix[prefix.length - 1] !== '/') {
// Some of the logic further down makes assumptions about the import depth.
throw Error(`Prefix "${prefix}" has to end in a slash.`);
Expand All @@ -95,6 +99,10 @@ function detectImports(content: string, prefix: string): DetectImportResult {
while (match = pattern.exec(content)) {
const [fullImport, type] = match;

if (excludedImports?.test(fullImport)) {
continue;
}

if (type === 'use') {
const namespace = extractNamespaceFromUseStatement(fullImport);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ export class ThemingApiMigration extends DevkitMigration<null> {
if (extname(stylesheet.filePath) === '.scss') {
const content = stylesheet.content;
const migratedContent = content ? migrateFileContent(content,
'~@angular/material/', '~@angular/cdk/', '~@angular/material', '~@angular/cdk') : content;
'~@angular/material/', '~@angular/cdk/', '~@angular/material', '~@angular/cdk',
undefined, /material\/prebuilt-themes|cdk\/.*-prebuilt/) : content;
Comment thread
devversion marked this conversation as resolved.

if (migratedContent && migratedContent !== content) {
this.fileSystem.edit(stylesheet.filePath)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -716,4 +716,22 @@ describe('v12 theming API migration', () => {
`@include mat.mdc-button-theme();`,
].join('\n'));
});

it('should not drop imports of prebuilt styles', async () => {
writeLines(THEME_PATH, [
`@import '~@angular/material/prebuilt-themes/indigo-pink.css';`,
`@import '~@angular/material/theming';`,
`@import '~@angular/cdk/overlay-prebuilt.css';`,
`@include mat-core();`,
]);

await runMigration();

expect(splitFile(THEME_PATH)).toEqual([
`@use '~@angular/material' as mat;`,
`@import '~@angular/material/prebuilt-themes/indigo-pink.css';`,
`@import '~@angular/cdk/overlay-prebuilt.css';`,
Comment on lines +732 to +733
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be possible to migrate these to the following

Suggested change
`@import '~@angular/material/prebuilt-themes/indigo-pink.css';`,
`@import '~@angular/cdk/overlay-prebuilt.css';`,
`@use '@angular/material/prebuilt-themes/indigo-pink.css';`,
`@use '@angular/cdk/overlay-prebuilt.css';`,

Or is that something that you are depending upon the sass-migrator to do?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that imports of .css files are still supposed to go through @import.

Copy link
Copy Markdown
Contributor

@Splaktar Splaktar May 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that too at first, but found that they work fine with @use as long as the deprecated sass-loader feature of using ~ is not used.

If you've seen an official statement from the Sass team to the otherwise, please do let me know.

`@include mat.core();`,
]);
});
});