fix(compiler-cli): Return original sourceFile instead of redirected sourceFile #26036

JonWallsten · 2018-09-20T12:32:00Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

When using webpack dev server and AngularCompilerPlugin (maybe other combinations as well) all but the first compile always fails with the the following error:

Host should not return a redirect source file from `getSourceFile`

The problem is causes by duplicated definition files with different paths.
Example:

C:/Users/me/repo/web-app-angular/node_modules/@types/webpack/node_modules/source-map/source-map.d.ts
C:/Users/me/repo/web-app-angular/node_modules/@types/uglify-js/node_modules/source-map/source-map.d.ts

Typescript expects the redirect's target instead of the redirects source.
This fix solves that.
Issue Number: #22524

What is the new behavior?

The error message is not thrown. Fix is borrowed from here: ng-packagr/ng-packagr@c1fced0

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

This is definitely a hotfix. I don't have enough knowledge about TypeScript to solve the root cause. But this has been stopping our project since March, and based ont he up votes on the issue I'm not the only one.

JonWallsten · 2018-09-21T05:27:36Z

Build seems to be failing because:

===== archive - skip deploy - commit not HEAD
fatal: unable to access 'https://github.com/angular/angular.git/': gnutls_handshake() failed: A TLS packet with unexpected length was received.
Fail

yannickglt · 2018-09-26T07:22:37Z

I confirm that it fixes watch on server. Is there a chance it get merged and released on an angular patch soon?

JonWallsten · 2018-10-07T18:49:32Z

Is this just going to be ignored forever?

JonWallsten · 2019-07-04T10:06:54Z

Please note that this needs to be fixed in for IVY as well:
https://github.com/angular/angular/blob/master/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts#L176

Splaktar · 2019-07-04T22:33:44Z

@JonWallsten sorry for the delay on this. It should start moving forward now.

Would it be possible to post a PR with the Ivy fix as well?

JonWallsten · 2019-07-05T04:15:28Z

@Splaktar Sure! I'll get one done by this weekend.

JonWallsten · 2019-07-06T07:37:53Z

@Splaktar: I've updated the PR now. However, I don't know if there are more places. These are the two I've come across in my current project while bulding with Webpack. I did dig around some in the project but couldn't find any obvious ones. But then again. It's not crystal clear code-wise where this is an issue. There are around 40 places where getSourceFiles is used, but not all of them uses the source files.

JonWallsten · 2019-07-09T07:25:32Z

@JoostK: Is it possible to trigger tests again without a new commit?

petebacondarwin · 2019-07-09T07:49:43Z

@JonWallsten - hi Jon. Members of the angular org can retrigger CircleCI tests (possibly the PR author themselves can) by going to the CircleCI UI. CodeFresh doesn't have this ability (AFAIK) so that can only be re-run by a new commit.
A simple way to push a new run is git commit --amend --reuse-message=HEAD && git push --force-with-lease. Hope I am not teaching you something you already know there!

JonWallsten · 2019-07-09T08:34:47Z

@petebacondarwin: That's a new one! Will be usefull! Thanks!

JonWallsten · 2019-07-09T08:54:27Z

@JoostK @petebacondarwin - the build seems to fail. And I have no idea what the error message means.

JoostK · 2019-07-09T08:58:04Z

@JonWallsten neither do I, it seems like a flake. I restarted the workflow for you, lets hope that resolves the issue.

petebacondarwin · 2019-07-09T08:59:27Z

LOL! So did I. I'll leave @JoostK to help out here. I think it was probably a flake.

JonWallsten · 2019-07-09T09:30:22Z

@JoostK @petebacondarwin: Weird! Happends twice in a row. That's why I thought it was a real error. But third times the charm it seems! Thanks guys!

JonWallsten · 2019-07-09T10:23:37Z

@JoostK: Let me know if there's anything more you'd need me to do!

JoostK · 2019-07-09T10:49:10Z

packages/compiler-cli/src/transformers/compiler_host.ts

+        // location.
+        // We return 'redirectInfo.unredirected' to return the original file and not the, by
+        // TypeScript, suggested target file.
+        if ((summary.sourceFile as any).redirectInfo) {


I would prefer the code to read as follows:

sf = summary.sourceFile; const redirectInfo = (sf as any).redirectInfo; if (redirectInfo !== undefined) { sf = redirectInfo.unredirected; }

JoostK · 2019-07-09T11:00:07Z

packages/compiler-cli/src/ngtsc/typecheck/src/host.ts

@@ -39,6 +39,12 @@ export class TypeCheckProgramHost implements ts.CompilerHost {
      sf = this.delegate.getSourceFile(
          fileName, languageVersion, onError, shouldCreateNewSourceFile);
      sf && this.sfMap.set(fileName, sf);
+    } else if ((sf as any).redirectInfo) {


Also here, I think the following reads better:

} else { const redirectInfo = (sf as any).redirectInfo; if (redirectInfo !== undefined) { sf = redirectInfo.unredirected; } }

Maybe the comment should mention that offering the redirect source file is not allowed by TypeScript, hence the effort to obtain the original source file.

…ourceFile from getSourceFile Closes angular#22524

JonWallsten · 2019-07-09T17:13:55Z

@JoostK: Done! Probably what have taken you much less time to do this yourself. I feel like a middleman! ;)
But I appreciate your patience.

JoostK · 2019-07-09T17:35:01Z

@JonWallsten thank you for your perseverance! It is much appreciated 👍The compiler master @alxhub is not available this week, but I am pretty confident we can get this approved and merged soon.

Splaktar · 2019-07-09T22:54:15Z

@JonWallsten many thanks for your contribution here!

alxhub

👍

JoostK · 2019-07-15T19:56:00Z

merge-assistance: please run presubmit

matsko · 2019-07-15T20:43:02Z

https://test.corp.google.com/ui#id=OCL:258225401:BASE:258226709:1563222740807:8b7dd4e

…ourceFile from getSourceFile (#26036) Closes #22524 PR Close #26036

jorroll · 2019-07-21T12:52:36Z

That moment when you first encounter a really annoying Angular bug, only to realize that other people have been dealing with it for over a year and just fixed it for you.

Thank you @JonWallsten!!!!!! 🎉 🎉 🎊

…ourceFile from getSourceFile (angular#26036) Closes angular#22524 PR Close angular#26036

angular-automatic-lock-bot · 2019-09-15T08:21:11Z

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.}

googlebot added the cla: yes label Sep 20, 2018

JonWallsten force-pushed the patch-issue-22524 branch 4 times, most recently from 42f8e6e to 800b878 Compare September 20, 2018 13:46

JonWallsten changed the title ~~Patch issue 22524~~ fix(compiler-cli): Temp fix in program.ts issue Sep 20, 2018

JonWallsten force-pushed the patch-issue-22524 branch from 800b878 to ded09d3 Compare September 21, 2018 06:00

kara added the area: core Issues related to the framework runtime label Sep 24, 2018

ngbot bot added this to the needsTriage milestone Jan 24, 2019

JonWallsten mentioned this pull request Feb 28, 2019

Host should not return a redirect source file from getSourceFile #22524

Closed

petebacondarwin requested a review from alxhub July 4, 2019 09:51

petebacondarwin added area: compiler Issues related to `ngc`, Angular's template compiler comp: ivy action: review The PR is still awaiting reviews from at least one requested reviewer type: bug/fix labels Jul 4, 2019

petebacondarwin approved these changes Jul 4, 2019

View reviewed changes

kara added action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jul 4, 2019

JonWallsten force-pushed the patch-issue-22524 branch from ded09d3 to 8b972a8 Compare July 6, 2019 06:36

JonWallsten requested a review from a team as a code owner July 6, 2019 06:36

JonWallsten force-pushed the patch-issue-22524 branch from 8b972a8 to d111b23 Compare July 6, 2019 10:03

JonWallsten force-pushed the patch-issue-22524 branch from 08df399 to 70f37a8 Compare July 9, 2019 08:33

JonWallsten force-pushed the patch-issue-22524 branch from 70f37a8 to cc33dc0 Compare July 9, 2019 09:34

JoostK reviewed Jul 9, 2019

View reviewed changes

JonWallsten force-pushed the patch-issue-22524 branch 2 times, most recently from 64361a2 to 5bd001a Compare July 9, 2019 16:57

fix(compiler-cli): Return original sourceFile instead of redirected s…

9bd0e96

…ourceFile from getSourceFile Closes angular#22524

JonWallsten force-pushed the patch-issue-22524 branch from 5bd001a to 9bd0e96 Compare July 9, 2019 17:12

alxhub approved these changes Jul 15, 2019

View reviewed changes

JoostK added target: patch This PR is targeted for the next patch release merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Jul 15, 2019

matsko closed this in 3166cff Jul 15, 2019

matsko pushed a commit that referenced this pull request Jul 15, 2019

fix(compiler-cli): Return original sourceFile instead of redirected s…

13dbb98

…ourceFile from getSourceFile (#26036) Closes #22524 PR Close #26036

JonWallsten deleted the patch-issue-22524 branch July 18, 2019 09:20

sabeersulaiman pushed a commit to sabeersulaiman/angular that referenced this pull request Sep 6, 2019

fix(compiler-cli): Return original sourceFile instead of redirected s…

8407e6c

…ourceFile from getSourceFile (angular#26036) Closes angular#22524 PR Close angular#26036

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 15, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(compiler-cli): Return original sourceFile instead of redirected sourceFile #26036

fix(compiler-cli): Return original sourceFile instead of redirected sourceFile #26036

JonWallsten commented Sep 20, 2018 •

edited

JonWallsten commented Sep 21, 2018

yannickglt commented Sep 26, 2018

JonWallsten commented Oct 7, 2018

JonWallsten commented Jul 4, 2019

Splaktar commented Jul 4, 2019

JonWallsten commented Jul 5, 2019

JonWallsten commented Jul 6, 2019 •

edited

JonWallsten commented Jul 9, 2019

petebacondarwin commented Jul 9, 2019

JonWallsten commented Jul 9, 2019

JonWallsten commented Jul 9, 2019

JoostK commented Jul 9, 2019

petebacondarwin commented Jul 9, 2019

JonWallsten commented Jul 9, 2019

JonWallsten commented Jul 9, 2019

JoostK Jul 9, 2019

JoostK Jul 9, 2019

JonWallsten commented Jul 9, 2019 •

edited

JoostK commented Jul 9, 2019

Splaktar commented Jul 9, 2019

alxhub left a comment

JoostK commented Jul 15, 2019

matsko commented Jul 15, 2019

jorroll commented Jul 21, 2019

angular-automatic-lock-bot bot commented Sep 15, 2019

fix(compiler-cli): Return original sourceFile instead of redirected sourceFile #26036

fix(compiler-cli): Return original sourceFile instead of redirected sourceFile #26036

Conversation

JonWallsten commented Sep 20, 2018 • edited

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

JonWallsten commented Sep 21, 2018

yannickglt commented Sep 26, 2018

JonWallsten commented Oct 7, 2018

JonWallsten commented Jul 4, 2019

Splaktar commented Jul 4, 2019

JonWallsten commented Jul 5, 2019

JonWallsten commented Jul 6, 2019 • edited

JonWallsten commented Jul 9, 2019

petebacondarwin commented Jul 9, 2019

JonWallsten commented Jul 9, 2019

JonWallsten commented Jul 9, 2019

JoostK commented Jul 9, 2019

petebacondarwin commented Jul 9, 2019

JonWallsten commented Jul 9, 2019

JonWallsten commented Jul 9, 2019

JoostK Jul 9, 2019

Choose a reason for hiding this comment

JoostK Jul 9, 2019

Choose a reason for hiding this comment

JonWallsten commented Jul 9, 2019 • edited

JoostK commented Jul 9, 2019

Splaktar commented Jul 9, 2019

alxhub left a comment

Choose a reason for hiding this comment

JoostK commented Jul 15, 2019

matsko commented Jul 15, 2019

jorroll commented Jul 21, 2019

angular-automatic-lock-bot bot commented Sep 15, 2019

JonWallsten commented Sep 20, 2018 •

edited

JonWallsten commented Jul 6, 2019 •

edited

JonWallsten commented Jul 9, 2019 •

edited