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

fix(compiler-cli): Allow analysis to continue with invalid style url #41403

Closed
wants to merge 2 commits into from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Mar 31, 2021

Currently, we throw a FatalDiagnosticError when we fail to load a resource
(templateUrl or styleUrl) at various stages in the compiler. This prevents
analysis of the component from completing. This will result in in users not being
able to get any information in the component template when there is a missing
styleUrl, for example.

This commit simply tracks the diagnostic, marks the component as poisoned, and
continues merrily along. Environments configured to use poisoned data
(like the language service) will then be able to use other information from the analysis.

Fixes angular/vscode-ng-language-service#1241

@atscott atscott added state: WIP area: compiler Issues related to `ngc`, Angular's template compiler labels Mar 31, 2021
@ngbot ngbot bot modified the milestone: Backlog Mar 31, 2021
@google-cla google-cla bot added the cla: yes label Mar 31, 2021
@atscott atscott added target: patch This PR is targeted for the next patch release and removed cla: yes labels Mar 31, 2021
@pullapprove pullapprove bot requested a review from alxhub March 31, 2021 21:28
@google-cla google-cla bot added the cla: yes label Mar 31, 2021
@atscott
Copy link
Contributor Author

atscott commented Apr 1, 2021

presubmit

@atscott atscott added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP labels Apr 1, 2021
Currently, we throw a FatalDiagnosticError when we fail to load a resource
(`templateUrl` or `styleUrl`) at various stages in the compiler. This prevents
analysis of the component from completing. This will result in in users not being
able to get any information in the component template when there is a missing
`styleUrl`, for example.

This commit simply tracks the diagnostic, marks the component as poisoned, and
continues merrily along. Environments configured to use poisoned data
(like the language service) will then be able to use other information from the analysis.

Fixes angular/vscode-ng-language-service#1241
styleUrl.url, containingFile, styleUrl.nodeForError, resourceType);
const resourceStr = this.resourceLoader.load(resourceUrl);
try {
const resourceUrl = this._resolveResourceOrThrow(
Copy link
Member

Choose a reason for hiding this comment

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

Rather than this try/catch, we should make _resolveResourceOrThrow not throw, and instead return some kind of error that we can choose to deal with (or ignore) as we see fit. Possibly this means its return value will be some kind of union between a success type and a failure type that also includes a diagnostic.

diagnostics = [];
}
diagnostics.push(makeDiagnostic(e.code, e.node, e.message, e.relatedInformation));
isPoisoned = true;
Copy link
Member

Choose a reason for hiding this comment

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

isPoisoned means "cannot be relied upon by consuming components" - a component shouldn't be poisoned because its own template or stylesheets are unavailable as they're not a part of its public API.

@atscott atscott force-pushed the badcssresource branch 2 times, most recently from 64aa114 to 26fac59 Compare April 5, 2021 22:03
@atscott atscott added action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release and removed action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Apr 7, 2021
@atscott atscott closed this in 8f12f47 Apr 7, 2021
atscott added a commit to atscott/angular that referenced this pull request Apr 7, 2021
…ngular#41403)

Currently, we throw a FatalDiagnosticError when we fail to load a resource
(`templateUrl` or `styleUrl`) at various stages in the compiler. This prevents
analysis of the component from completing. This will result in in users not being
able to get any information in the component template when there is a missing
`styleUrl`, for example.

This commit simply tracks the diagnostic, marks the component as poisoned, and
continues merrily along. Environments configured to use poisoned data
(like the language service) will then be able to use other information from the analysis.

Fixes angular/vscode-ng-language-service#1241

PR Close angular#41403
atscott added a commit to atscott/angular that referenced this pull request Apr 7, 2021
…ngular#41403)

Currently, we throw a FatalDiagnosticError when we fail to load a resource
(`templateUrl` or `styleUrl`) at various stages in the compiler. This prevents
analysis of the component from completing. This will result in in users not being
able to get any information in the component template when there is a missing
`styleUrl`, for example.

This commit simply tracks the diagnostic, marks the component as poisoned, and
continues merrily along. Environments configured to use poisoned data
(like the language service) will then be able to use other information from the analysis.

Fixes angular/vscode-ng-language-service#1241

PR Close angular#41403
atscott added a commit to atscott/angular that referenced this pull request Apr 7, 2021
…ngular#41403)

Currently, we throw a FatalDiagnosticError when we fail to load a resource
(`templateUrl` or `styleUrl`) at various stages in the compiler. This prevents
analysis of the component from completing. This will result in in users not being
able to get any information in the component template when there is a missing
`styleUrl`, for example.

This commit simply tracks the diagnostic, marks the component as poisoned, and
continues merrily along. Environments configured to use poisoned data
(like the language service) will then be able to use other information from the analysis.

Fixes angular/vscode-ng-language-service#1241

PR Close angular#41403
atscott added a commit that referenced this pull request Apr 7, 2021
…41403) (#41489)

Currently, we throw a FatalDiagnosticError when we fail to load a resource
(`templateUrl` or `styleUrl`) at various stages in the compiler. This prevents
analysis of the component from completing. This will result in in users not being
able to get any information in the component template when there is a missing
`styleUrl`, for example.

This commit simply tracks the diagnostic, marks the component as poisoned, and
continues merrily along. Environments configured to use poisoned data
(like the language service) will then be able to use other information from the analysis.

Fixes angular/vscode-ng-language-service#1241

PR Close #41403

PR Close #41489
@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 May 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler cla: yes target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Be more tolerant of missing resources
2 participants