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

Update the webpack configuration in Angular Plugin #19

Merged
merged 2 commits into from
Nov 9, 2018

Conversation

AndSviat
Copy link
Contributor

@AndSviat AndSviat commented Nov 7, 2018

The workaround with webpack.ContextReplacementPlugin for angular/angular#11580 doesn’t really work for Angular 4, 5, 6, and 7:

Warning #1: https://prnt.sc/lezevh
Warning #2: https://prnt.sc/lezfmd

The update brings the changes in AngularPlugin.ts to suppress the warnings.

@AndSviat AndSviat requested a review from larixer November 7, 2018 16:25
* It's necessary to remove the @angular part and fix the issue by enabling
* the proper module.rules.Rule for @angular/core/*.js files(see above).
*/
/angular[\\\/]core/,
path.join(builder.require.cwd, 'src'),
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need to append src here? Could you check Svyat? I think we don't need to make assumption that all the package source code will be inside src and everything will work even if we don't append src here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I am concerned, we can remove the last two arguments in webpack.ContextReplacementPlugin and just pass the correct regexp angular[\\\/]core.
Tested the project with webpack.ContextReplacementPlugin(/angular[\\\/]core/) for the Angular ^4, ^5, ^6, ^7 versions.

@larixer
Copy link
Member

larixer commented Nov 9, 2018

Many thanks @AndSviat !

@larixer larixer merged commit c374431 into master Nov 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants