Skip to content

fix(@angular-devkit/build-angular): add sourceMappingURL comment for … #16524

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
Jan 2, 2020
Merged

fix(@angular-devkit/build-angular): add sourceMappingURL comment for … #16524

merged 1 commit into from
Jan 2, 2020

Conversation

alan-agius4
Copy link
Collaborator

…ES2015 during differential loading

When having differential loading enabled we only add the sourceMappingURL comment when optimization is enabled, because we only process these bundles when we enabling optimization.

With this change we now process such bundles even when optimization is disabled and add sourceMappingURL when source maps are enabled and not hidden.

Closes #16522

@alan-agius4 alan-agius4 added PR target: LTS-only target: patch This PR is targeted for the next patch release labels Jan 2, 2020
@alan-agius4 alan-agius4 requested a review from clydin January 2, 2020 10:21
@alan-agius4 alan-agius4 closed this Jan 2, 2020
@alan-agius4 alan-agius4 reopened this Jan 2, 2020
Copy link
Member

@clydin clydin left a comment

Choose a reason for hiding this comment

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

One change request otherwise looks good.

result.downlevel = await processBundle({
...options,
code: downlevelCode,
map: downlevelMap && JSON.stringify(downlevelMap),
Copy link
Member

Choose a reason for hiding this comment

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

Can map be a string or a RawSourceMap here with a check in the processBundle function to handle either? Sourcemaps can easily be in the megabyte range. Serializing and immediately deserializing can have performance and memory usage concerns.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah make sense! Done.

@clydin
Copy link
Member

clydin commented Jan 2, 2020

Also, this will most likely not apply cleanly to 8.3.x so a more minimal change set may be needed for that branch.

…ES2015 during differential loading

When having differential loading enabled we only add the `sourceMappingURL` comment when optimization is enabled, because we only process these bundles when we enabling optimization.

With this change we now process such bundles even when optimization is disabled and add `sourceMappingURL` when source maps are enabled and not hidden.

Closes #16522
@alan-agius4
Copy link
Collaborator Author

Blocking until the LTS version is green. As I am think it might be related to caching.

@vikerman vikerman merged commit 490c24b into angular:master Jan 2, 2020
@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 Feb 2, 2020
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.

Angular 8.3.21, ng build with target es 2015 doesn't show sourcemapped ts/css files in browser for debugging
4 participants