-
Notifications
You must be signed in to change notification settings - Fork 12k
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(@angular/build): remove Vite "/@id/" prefix for explicit external dependencies #28039
fix(@angular/build): remove Vite "/@id/" prefix for explicit external dependencies #28039
Conversation
0d14195
to
6feabeb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach appears viable as a workaround.
I left several initial comments.
There is the potential for a false positive replacement. However, the likelihood is very low and would only occur if externalDependencies
is used which is not overly common in its own. So I think that is acceptable as a (hopefully) temporary workaround.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach appears viable as a workaround.
I left several initial comments.
There is the potential for a false positive replacement. However, the likelihood is very low and would only occur if externalDependencies
is used which is not overly common in its own. So I think that is acceptable as a (hopefully) temporary workaround.
6feabeb
to
9ddef76
Compare
Also added a test now. Let me know if you have more suggestions or concerns. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one comment otherwise LGTM.
Thank you for the contribution.
9ddef76
to
5d2320e
Compare
… dependencies Adds a Vite plugin which will remove the /@id/ prefix (which gets inserted by Vite during import-analysis) for explicit externalDependencies.
5d2320e
to
9212c77
Compare
Thank you for the fast reviews! |
The changes were merged into the following branches: main, 18.1.x |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This PR adds a Vite plugin to remove the
/@id/
prefix (which gets inserted by Vite during import-analysis) for explicit externalDependencies.PR Checklist
Please check to confirm your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #28028
What is the new behavior?
The
/@id/
prefix gets removed from a new Vite plugin.Does this PR introduce a breaking change?
Other information