Angular: Support v19.2 when @angular/animations is not installed #30611
Conversation
valentinpalkovic
added
ci:daily
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
10 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile
| @@ -71,8 +63,8 @@ export class PropertyExtractor implements NgModuleMetadata { | |||
| } | |||
| } | |||
|
|
|||
| private init() { | |||
| const analyzed = this.analyzeMetadata(this.metadata); | |||
| public async init() { | |||
There was a problem hiding this comment.
logic: init() is now public and async but constructor no longer calls it - this could cause issues if callers forget to call init()
There was a problem hiding this comment.
instead of requiring everyone to call PropertyExtractor.init() manually after constructing, maybe you can do:
constructor(
private metadata: NgModuleMetadata,
private component?: any
) {
return this.init().then(() => this);
}
so you can do await new PropertyExtractor(), that will run init and return the instance.
| ); | ||
| return [true, provideNoopAnimations()]; | ||
| if (ngModule === animations.NoopAnimationsModule) { | ||
| console.error( |
There was a problem hiding this comment.
style: console.error() used for NoopAnimationsModule warning while other similar warnings use console.warn() - should be consistent
| console.error( | |
| console.warn( |
| try { | ||
| await import('@angular/platform-browser/animations'); | ||
| } catch (e) { | ||
| webpackConfig.plugins.push( | ||
| new WebpackIgnorePlugin({ | ||
| resourceRegExp: /@angular\/platform-browser\/animations$/, | ||
| }) | ||
| ); | ||
| } |
There was a problem hiding this comment.
style: Consider catching a more specific error type than just 'e' to avoid accidentally catching unrelated errors
There was a problem hiding this comment.
logic: Removing this template file will break the OpenCloseComponent and its stories unless corresponding changes are made to move this template inline or provide an alternative.
valentinpalkovic
added
the
patch:yes
|
View your CI Pipeline Execution ↗ for commit 46eb952.
☁️ Nx Cloud last updated this comment at |
valentinpalkovic
force-pushed
the
valentin/support-angular-19.2
branch
from
82faba9 to
d0b1713
Compare
valentinpalkovic
force-pushed
the
valentin/support-angular-19.2
branch
from
854b7a6 to
46eb952
Compare
| @@ -71,8 +63,8 @@ export class PropertyExtractor implements NgModuleMetadata { | |||
| } | |||
| } | |||
|
|
|||
| private init() { | |||
| const analyzed = this.analyzeMetadata(this.metadata); | |||
| public async init() { | |||
There was a problem hiding this comment.
instead of requiring everyone to call PropertyExtractor.init() manually after constructing, maybe you can do:
constructor(
private metadata: NgModuleMetadata,
private component?: any
) {
return this.init().then(() => this);
}
so you can do await new PropertyExtractor(), that will run init and return the instance.
|
That's correct. But I have decided against it after reading https://dev.to/somedood/the-proper-way-to-write-async-constructors-in-javascript-1o8c |
Closes #
What I did
In the current release candidate of Angular (v19.2.0-rc.0) it doesn't install
@angular/animationsper default when initializing Angular freshly. Storybook, though, had some imports on@angular/animationswhich started to fail.Now, Storybook doesn't require having
@angular/animationsbeing present, but conditionally applies logic when it is.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>Greptile Summary
Here's my analysis of the pull request:
Makes Angular renderer compatible with Angular v19.2 by making @angular/animations optional, focusing on dynamic imports and conditional animation handling.
AbstractRenderer.tsforPropertyExtractor.init()to handle conditional animation loadingframework-preset-angular-cli.tsto conditionally import animations and addWebpackIgnorePluginwhen module is missing@angular/animationsan optional peer dependency inpackage.jsonwith version range>=15.0.0 < 20.0.0PropertyExtractor.tsto handle standalone components defaulting to true in Angular 19+