Skip to content

fix(@angular-devkit/build-angular): only add module script types to actual module scripts #14985

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

Merged
merged 1 commit into from
Jul 9, 2019

Conversation

clydin
Copy link
Member

@clydin clydin commented Jul 3, 2019

ng serve was errantly adding a module type to custom script bundles. These scripts may contain ES5 and non-strict compatible code and can therefore not be marked as module scripts. ng build already correctly operates in this fashion.

Fixes #14952

@clydin clydin added the target: patch This PR is targeted for the next patch release label Jul 3, 2019
@clydin clydin requested a review from alan-agius4 July 3, 2019 15:35
…ctual module scripts

`ng serve` was errantly adding a module type to custom script bundles.  These scripts may contain ES5 and non-strict compatible code and can therefore not be marked as module scripts.  `ng build` already correctly operates in this fashion.

Fixes angular#14952
@clydin clydin force-pushed the fix-server-script-module branch from 95f4e81 to 34e55c6 Compare July 3, 2019 16:09
@mgechev
Copy link
Member

mgechev commented Jul 3, 2019

Third-party dependencies part of the main bundle might not be strict mode compatible either. My concern with such fix is that we introduce different behavior for the scripts based on how the application depends on them.

Looks like the behavior is already inconsistent given that we inject third-party scripts as modules in development but not in production.

@clydin
Copy link
Member Author

clydin commented Jul 3, 2019

It’s actually already different. Build operates this way and serve did as well up until 8.1. Serve behavior was changed due to a fix for the main bundles that unintentionally changed this as well.

This also allows people to use those third party libraries that would otherwise be unable to be included in the main application bundles, if needed. Without it, there would be no clean way to do so.

Copy link
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

Make sense to me. LGTM.

This does indeed align the behaviour between serve and build more.

@mgechev
Copy link
Member

mgechev commented Jul 3, 2019

Sounds good

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot access variable that is initialized when jQuery is ready
5 participants