Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 of 15 tasks
msieurtoph opened this issue May 13, 2021 · 4 comments · Fixed by #22698
Assignees
Labels
area: ng-update Issues related to `ng-update` integration P2 The issue is important to a large percentage of users, with a workaround

Comments

@msieurtoph
Copy link

🐞 Bug report

Command (mark with an x)

  • new
  • build
  • serve
  • test
  • e2e
  • generate
  • add
  • update
  • lint
  • extract-i18n
  • run
  • config
  • help
  • version
  • doc

Is this a regression?

I have not seen this bug during previous migrations

Description

During the update of Material from 11 to 12, the link to the theme has been automatically removed from the style.scss file (aka
this line has been removed : @import "~@angular/material/prebuilt-themes/indigo-pink.css")
But it has not been automatically added back to the styles array of the angular.json.

🔬 Minimal Reproduction

(sorry I don't know the command line for each step, specially the first one that needs to create a project in version 11)

  1. Create new project in version 11 with the cli and add Material support.
  2. Add a theme to the style.scss (@import "~@angular/material/prebuilt-themes/indigo-pink.css")
  3. Launch the ng update @angular/material

You will see the line has been removed from the styles.scss file ... but no add to styles arry of the angular.json file.

🔥 Exception or Error




// no error is returned

🌍 Your Environment




$ ng version

     _                      _                 ____ _     ___
    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/


Angular CLI: 12.0.0
Node: 14.15.1
Package Manager: npm 6.14.8
OS: win32 x64

Angular: 12.0.0
... animations, cdk, cli, common, compiler, compiler-cli, core
... forms, language-service, material, material-moment-adapter
... platform-browser, platform-browser-dynamic, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1200.0
@angular-devkit/build-angular   12.0.0
@angular-devkit/core            12.0.0
@angular-devkit/schematics      12.0.0
@schematics/angular             12.0.0
rxjs                            7.0.1
typescript                      4.2.4

Anything else relevant?

@petebacondarwin petebacondarwin transferred this issue from angular/angular-cli May 14, 2021
@crisbeto crisbeto self-assigned this May 14, 2021
crisbeto added a commit to crisbeto/material2 that referenced this issue May 14, 2021
…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
Copy link
Member

Thank you for the report, it looks like the migration is a bit too aggressive. I've submitted #22698 to resolve it, but you should also be able to re-add the import manually yourself.

@crisbeto crisbeto added area: ng-update Issues related to `ng-update` integration has pr P2 The issue is important to a large percentage of users, with a workaround labels May 14, 2021
@msieurtoph
Copy link
Author

msieurtoph commented May 14, 2021

Thanks for the PR.

... but you should also be able to re-add the import manually yourself.

yeah, that's what I did first ... before to figure out it should be in the angular.json instead.
But I guess the update engine could easily copy it from the styles.scss to the angular.json too.

@Splaktar
Copy link
Member

Ah, this is one of the issues that I ran into last night as well. Thank you for the quick fix @crisbeto.

andrewseguin pushed a commit that referenced this issue May 18, 2021
…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 #22697.
@angular-automatic-lock-bot
Copy link

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
area: ng-update Issues related to `ng-update` integration P2 The issue is important to a large percentage of users, with a workaround
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants