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

fix(material/core): prevent updates to v17 if project uses legacy components #28024

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Oct 31, 2023

These changes add a schematic that will log a fatal error and prevent the app from updating to v17 if it's using legacy components. Legacy components have been deleted in v17 so the app won't build if it updates.

Updating to Material v17 using ng update will look like this:
image

@crisbeto crisbeto added P2 The issue is important to a large percentage of users, with a workaround target: rc This PR is targeted for the next release-candidate labels Oct 31, 2023
@crisbeto crisbeto force-pushed the legacy-imports-error branch 3 times, most recently from 52f1362 to b238105 Compare October 31, 2023 08:55
@crisbeto crisbeto marked this pull request as ready for review October 31, 2023 09:02

if (packageJson !== null && packageJson['dependencies']) {
packageJson['dependencies']['@angular/material'] = '^16.2.0';
tree.overwrite('package.json', JSON.stringify(packageJson, null, 2));
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that we have a utility for adding dependencies to the package.json (in at least two places!), however my understanding is that we can't put it into a common spot, because the other places are ng add schematics and they might not be able to import other packages yet (I may be wrong). This operation is pretty basic so we should be fine.

* from running if the project is using legacy components.
* @param onSuccess Rule to run if there are no legacy imports.
*/
export function legacyImportsError(onSuccess: Rule): Rule {
Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't go through the DevkitMigration for this schematic, because it isn't a standard migration schematic and the operations in it are pretty basic so it wasn't worth it to update the common infrastructure.

@crisbeto crisbeto added this to the 17.0.0 milestone Oct 31, 2023
: files.join('\n');

return (
`Cannot update to Angular Material v17, because the project is using the legacy ` +
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: remove the commas before "because" (and in the next sentence)

@crisbeto crisbeto self-assigned this Nov 1, 2023
@crisbeto crisbeto added the action: merge The PR is ready for merge by the caretaker label Nov 1, 2023
…ponents

These changes add a schematic that will log a fatal error and prevent the app from updating to v17 if it's using legacy components. Legacy components have been deleted in v17 so the app won't build if it updates.
@crisbeto crisbeto merged commit f991425 into angular:main Nov 1, 2023
20 of 23 checks passed
crisbeto added a commit that referenced this pull request Nov 1, 2023
…ponents (#28024)

These changes add a schematic that will log a fatal error and prevent the app from updating to v17 if it's using legacy components. Legacy components have been deleted in v17 so the app won't build if it updates.

(cherry picked from commit f991425)
@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 Dec 2, 2023
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 P2 The issue is important to a large percentage of users, with a workaround target: rc This PR is targeted for the next release-candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants