Angular: switch ɵReflectionCapabilities -> reflectComponentType in NgComponentAnalyzer #30954
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
code/frameworks/angular/src/client/angular-beta/utils/NgComponentAnalyzer.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
LGTM
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
vasimi
changed the title
There was a problem hiding this comment.
LGTM
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
…onentAnalyzer where possible, cover with unit tests
…entAnalyzer.ts Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
vasimi
force-pushed
the
switch-angular-io-to-reflectComponentType
branch
from
48c3b38 to
c01815e
Compare
|
Hi @vasimi Thank you for putting together the PR! Great stuff. Can you describe with some examples or create a GitHub issue of what the PR concretely fixes? |
|
View your CI Pipeline Execution ↗ for commit c456bbe.
☁️ Nx Cloud last updated this comment at |
|
While switching to reflectComponentType resolved the issue in my environment, I'm not able to come up with an example that would be broken in the same way as the actual code where I encountered this issue is (which I unfortunately cannot share) and even reflectComponentType apparently doesn't solve the issue in Storybook's unit tests (which would be failing before if signal inputs were tested at all). I'll be digging deeper into Angular and I'll try to get to the bottom of this. |
|
Hi @vasimi Can you take a look at the unit tests? https://app.circleci.com/pipelines/github/storybookjs/storybook/97236/workflows/ef65b294-8e52-4a3d-b325-0137255cbd72/jobs/817727 Some attributes are applied twice. |
|
Hi @vasimi, I have left a comment recently. Can you take a look? If you don't have enough free capacity left, I can take a look at it at some point. |
What I did
Replaced
ɵReflectionCapabilitieswithreflectComponentTypeinNgComponentAnalyzerwherever possible.ɵReflectionCapabilitiesdoes not return input/output signals, which is causing Storybook to set values directly to the component properties instead of updating them as inputs.It doesn't have any actual benefit in
isStandaloneComponent, but it felt better to use a documented API instead of something internal. In this function, I also removed the comment mentioning Angular < 14, since storybook already dropped support of versions that old.reflectComponentTypeis supported in Angular 15, the oldest version supported by Storybook.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
storybookjs/sandboxes#13
angular-cli/default-tstemplateDocumentation
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
This PR migrates the Angular component analyzer to use reflectComponentType instead of the internal ɵReflectionCapabilities, ensuring correct input/output signal extraction and updating related tests.
• Updated /code/frameworks/angular/src/client/angular-beta/utils/NgComponentAnalyzer.ts to safely call reflectComponentType(component)?.isStandalone.
• Adjusted /code/frameworks/angular/src/client/angular-beta/utils/NgComponentAnalyzer.test.ts to verify the new extraction behavior.
• Removed outdated Angular <14 comments from the isStandaloneComponent logic.
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!