-
Notifications
You must be signed in to change notification settings - Fork 26k
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(localize): add polyfill in polyfills array instead of polyfills.ts #47569
Conversation
|
This update is needed to implement the changes in `ng add localize` angular#47569
This update is needed to implement the changes in `ng add localize` angular#47569
- This update is needed to implement the changes in `ng add localize` angular#47569 - Add missing `root` options to all `angular.json`, this is required as otherwise the angular.json validation will fail. - Remove `require.context` from test.ts integration test, as this is no longer needed. - Update payloads golden files.
- This update is needed to implement the changes in `ng add localize` angular#47569 - Add missing `root` options to all `angular.json`, this is required as otherwise the angular.json validation will fail. - Remove `require.context` from test.ts integration test, as this is no longer needed. - Update payloads golden files.
- This update is needed to implement the changes in `ng add localize` angular#47569 - Add missing `root` options to all `angular.json`, this is required as otherwise the angular.json validation will fail. - Remove `require.context` from test.ts integration test, as this is no longer needed. - Update payloads golden files.
- This update is needed to implement the changes in `ng add localize` angular#47569 - Add missing `root` options to all `angular.json`, this is required as otherwise the angular.json validation will fail. - Remove `require.context` from test.ts integration test, as this is no longer needed. - Update payloads golden files.
- This update is needed to implement the changes in `ng add localize` #47569 - Add missing `root` options to all `angular.json`, this is required as otherwise the angular.json validation will fail. - Remove `require.context` from test.ts integration test, as this is no longer needed. - Update payloads golden files. PR Close #47584
With the recent changes in the Angular CLI (angular/angular-cli#23938) the polyfills option accepts module path that are resolved using Node module resolution. Also, the polyfills.ts file is no longer generated by default. This commit changes the way on how the polyfill is added to the projects.
517cc31
to
f78ea82
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.
@alan-agius4 the changes look good 👍
Quick question: do we have tests to cover error cases (file that we try to patch is not found, the value of the polyfills field is unexpected, etc)? Just curious what the DX would look like in those cases and if there is a need to improve error messages, etc (in separate PR(s)).
In general even during migrations we expect the project/builder target to be in a working state. This is because this is validated when running the CLI and an error would be displayed when there are invalid options. In the case a file is not found an error will be displayed. |
This PR was merged into the repository by commit 400a6b5. |
) - This update is needed to implement the changes in `ng add localize` angular#47569 - Add missing `root` options to all `angular.json`, this is required as otherwise the angular.json validation will fail. - Remove `require.context` from test.ts integration test, as this is no longer needed. - Update payloads golden files. PR Close angular#47584
angular#47569) With the recent changes in the Angular CLI (angular/angular-cli#23938) the polyfills option accepts module path that are resolved using Node module resolution. Also, the polyfills.ts file is no longer generated by default. This commit changes the way on how the polyfill is added to the projects. PR Close angular#47569
) - This update is needed to implement the changes in `ng add localize` angular#47569 - Add missing `root` options to all `angular.json`, this is required as otherwise the angular.json validation will fail. - Remove `require.context` from test.ts integration test, as this is no longer needed. - Update payloads golden files. PR Close angular#47584
angular#47569) With the recent changes in the Angular CLI (angular/angular-cli#23938) the polyfills option accepts module path that are resolved using Node module resolution. Also, the polyfills.ts file is no longer generated by default. This commit changes the way on how the polyfill is added to the projects. PR Close angular#47569
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. |
With the recent changes in the Angular CLI (angular/angular-cli#23938) the polyfills option accepts module path that are resolved using Node module resolution. Also, the polyfills.ts file is no longer generated by default.
This commit changes the way on how the polyfill is added to the projects.