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.

Sign up for GitHub

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

Jump to bottom

fix(compiler): always use ng:// prefix for sourcemap urls #15218

Merged
merged 1 commit into from
Mar 17, 2017
Merged

fix(compiler): always use ng:// prefix for sourcemap urls #15218

merged 1 commit into from
Mar 17, 2017

Conversation

tbosch
Copy link
Contributor

@tbosch tbosch commented Mar 16, 2017

Fixes:

  • In G3, filePaths don’t start with a / and therefore became relative.
  • Always using the ng:// prefix groups angular resources in the same
    way for AOT and JIT.

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x")

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

What is the new behavior?

Does this PR introduce a breaking change? (check one with "x")

[ ] Yes
[ ] No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

vicb
vicb reviewed Mar 16, 2017
View reviewed changes
packages/compiler/src/compile_metadata.ts Outdated
function ngJitFolder() {
return 'ng://';
export function sourceUrl(url: string) {
const urlWithourOrigin = url.replace(/(\w+:\/\/[\w:]+)?(\/+)?/, '');
Copy link
Contributor

Choose a reason for hiding this comment

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

add -

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

vicb
vicb reviewed Mar 16, 2017
View reviewed changes
packages/compiler/src/compile_metadata.ts Outdated
const urlWithourOrigin = url.replace(/(\w+:\/\/[\w:]+)?(\/+)?/, '');
// Note: We need 3 "/" so that ng shows up as a separate domain
// in the chrome dev tools.
return `ng:///${urlWithourOrigin}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

in the replace

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@tbosch
fix(compiler): always use ng:// prefix for sourcemap urls
3091267 
Fixes:
- In G3, filePaths don’t start with a `/` and therefore became relative.
- Always using the `ng://` prefix groups angular resources in the same
  way for AOT and JIT.
@tbosch tbosch added the action: merge The PR is ready for merge by the caretaker label Mar 16, 2017
@chuckjaz chuckjaz merged commit 994089d into angular:master Mar 17, 2017
@tbosch tbosch deleted the fix_sourceurls branch March 17, 2017 15:28
@boulix3
Copy link

boulix3 commented Mar 17, 2017

Hello,
I use this build sequence : ngc->rollup->sorcery
sorcery now fails with this error : Error: ENOENT: no such file or directory, open 'D:\github\my-demo-app\build\aot\src\app\ng:\D:\github\my-demo-app\src\app\app.component.ts'

I suppose it's due to the new prefix for sourcemap urls.

@tbosch
Copy link
Contributor Author

tbosch commented Mar 17, 2017

@boulix3 Could you try the following local modification in your node_modules for angular, so that I know the fix would work (as I don't have the right setup locally to reproduce):

  1. open node_modules/@angular/compiler/bundles/compiler.umd.js
  2. search for mapFirstOffsetIfNeeded, should find the following function:
            var /** @type {?} */ mapFirstOffsetIfNeeded = function () {
                if (!firstOffsetMapped) {
                    map.addSource(sourceFilePath).addMapping(0, sourceFilePath, 0, 0);
                    firstOffsetMapped = true;
                }
            };
  1. change map.addSource(sourceFilePath) to map.addSource(sourceFilePath, ' ') (i.e. add a string with a single space inside as second argument to addSource call.
  2. try again

@tbosch
Copy link
Contributor Author

tbosch commented Mar 17, 2017

@boulix3 FYI, sorcery seems to be less graceful with non existing sources (see e.g. Rich-Harris/sorcery#142)

@tbosch
Copy link
Contributor Author

tbosch commented Mar 17, 2017

See #15246

@boulix3
Copy link

boulix3 commented Mar 17, 2017

Thanks a lot for the fast reply!
Too bad I have to wait till monday at work to try it.

SamVerschueren pushed a commit to SamVerschueren/angular that referenced this pull request Mar 18, 2017
@tbosch @SamVerschueren
fix(compiler): always use ng:// prefix for sourcemap urls (angular#…
002dcab 
…15218)

Fixes:
- In G3, filePaths don’t start with a `/` and therefore became relative.
- Always using the `ng://` prefix groups angular resources in the same
  way for AOT and JIT.
@boulix3
Copy link

boulix3 commented Mar 20, 2017

I just finished testing your fix, it works great (I used 4.0.0-rc5).

@tbosch
Copy link
Contributor Author

tbosch commented Mar 20, 2017

@boulix3 nice, so I already made this change in Angular as well, will be part of the next rc.

asnowwolf pushed a commit to asnowwolf/angular that referenced this pull request Aug 11, 2017
@tbosch
fix(compiler): always use ng:// prefix for sourcemap urls (angular#…
b77d813 
…15218)

Fixes:
- In G3, filePaths don’t start with a `/` and therefore became relative.
- Always using the `ng://` prefix groups angular resources in the same
  way for AOT and JIT.
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017
@tbosch @juleskremer
fix(compiler): always use ng:// prefix for sourcemap urls (angular#…
378ec9a 
…15218)

Fixes:
- In G3, filePaths don’t start with a `/` and therefore became relative.
- Always using the `ng://` prefix groups angular resources in the same
  way for AOT and JIT.
@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 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@vicb vicb vicb approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tbosch @boulix3 @vicb @chuckjaz @googlebot