Skip to content

fix(material/schematics): don't drop prebuilt imports in theming API migration#22698

Merged
andrewseguin merged 1 commit intoangular:masterfrom
crisbeto:22697/prebuilt-themes-migration
May 18, 2021
Merged

fix(material/schematics): don't drop prebuilt imports in theming API migration#22698
andrewseguin merged 1 commit intoangular:masterfrom
crisbeto:22697/prebuilt-themes-migration

Conversation

@crisbeto
Copy link
Copy Markdown
Member

Currently the theming API migration drops any imports starting with ~@angular/material/, assuming that they're Sass APIs. This can result in prebuilt style imports being removed by mistake.

These changes add a regex based on which we'll exclude some imports from the migration.

Fixes #22697.

…migration

Currently the theming API migration drops any imports starting with `~@angular/material/`, assuming that they're Sass APIs. This can result in prebuilt style imports being removed by mistake.

These changes add a regex based on which we'll exclude some imports from the migration.

Fixes angular#22697.
@crisbeto crisbeto added P2 The issue is important to a large percentage of users, with a workaround merge safe target: patch This PR is targeted for the next patch release labels May 14, 2021
@crisbeto crisbeto requested a review from jelbourn May 14, 2021 11:03
@crisbeto crisbeto requested a review from devversion as a code owner May 14, 2021 11:03
@google-cla google-cla Bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label May 14, 2021
@crisbeto crisbeto added the action: merge The PR is ready for merge by the caretaker label May 14, 2021
Copy link
Copy Markdown
Contributor

@Splaktar Splaktar left a comment

Choose a reason for hiding this comment

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

Thank you for the quick PR on this!

Comment on lines +732 to +733
`@import '~@angular/material/prebuilt-themes/indigo-pink.css';`,
`@import '~@angular/cdk/overlay-prebuilt.css';`,
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.

@intellix
Copy link
Copy Markdown

intellix commented May 18, 2021

I'm not seeing any migrations/automatic updates happening when updating from 11 to 12 and I can't see any information anywhere as to when an update comes with a migration or not.

Is there a list somewhere? There's no mention of it in the release notes for example. I don't know if I should just manually go through my SCSS and update the references or if it should be automatic.

The release notes just seem to suggest re-reading the theming docs and doing it yourself:

For more information, check out the new theming guide.

@andrewseguin andrewseguin merged commit 7cd084d into angular:master May 18, 2021
@angular-automatic-lock-bot
Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Jun 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement P2 The issue is important to a large percentage of users, with a workaround target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Updating Material from 11 to 12 removes the theme in the styles.scss but forgets to add it back to the angular.json

5 participants