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

fixed NG0912 selector collision warnings #859

Merged
merged 3 commits into from
Jul 24, 2023
Merged

Conversation

jmcmichael
Copy link
Contributor

A few of these were identical selectors which were easily fixed with renaming. Most of the warnings were produced by the base-field mixin that all of the new form components use. All of those fields are compiling themselves using that same base class, and since it does not define a selector, NG gives it a default of 'ng-component'. The selector defined in the parent component's options does not override the base's selector, since those are set at compile-time (hence the requirement that selectors be strings and not variables or functions).

The solution turned out to be simple, replacing base-field's @Component decorator with @Injectable, which allows the use of @UntilDestroy within base-field and does not kick off NG's component compilation w/ its component collision testing. Now, the parent's component options aren't ignored, so its unique selector is used.

@jmcmichael jmcmichael requested a review from acoffman July 24, 2023 19:45
@github-actions
Copy link

You must specify one of the following labels: bugfix, housekeeping, new-feature, enhancement, ignore-for-release, dependencies in order for this PR to appear in the release notes.

@jmcmichael jmcmichael added the bugfix PR Label for bug fixes. Will appear in release notes. label Jul 24, 2023
Copy link
Member

@acoffman acoffman left a comment

Choose a reason for hiding this comment

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

Will the Let/Push rename conflict with the other open PR/did it come along accidentally? Or will those merge cleanly/does this supersede that one?

If its either of the later this looks good to me!

@jmcmichael jmcmichael changed the base branch from main to update-ng-16 July 24, 2023 19:51
@jmcmichael jmcmichael merged commit bf88edc into update-ng-16 Jul 24, 2023
3 of 5 checks passed
@jmcmichael jmcmichael deleted the fix-id-collisions branch July 24, 2023 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix PR Label for bug fixes. Will appear in release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants