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(language-service): getSourceFile() should only be called on TS files #31920

Closed
wants to merge 1 commit into from

Conversation

@kyliau
Copy link
Member

commented Jul 30, 2019

This is in preparation for Angular LS plugin handling external templates (HTML) files.
getSourceFile() parses the file and returns SourceFileObject, so it must never be called on HTML.
Otherwise, TS errors would show up in diagnostics.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • 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?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@kyliau kyliau requested a review from angular/tools-language-service as a code owner Jul 30, 2019

@ngbot ngbot bot modified the milestone: needsTriage Jul 30, 2019

@googlebot googlebot added the cla: yes label Jul 30, 2019

@kyliau kyliau force-pushed the kyliau:getTemplateAt branch 3 times, most recently from 46cec67 to 0a2711f Jul 31, 2019

@kyliau kyliau requested a review from ayazhafiz Jul 31, 2019

@kyliau kyliau force-pushed the kyliau:getTemplateAt branch from 0a2711f to 8f0210f Jul 31, 2019

@ayazhafiz
Copy link
Contributor

left a comment

lgtm

@ngbot

This comment has been minimized.

Copy link

commented Jul 31, 2019

I see that you just added the PR action: merge label, but the following checks are still failing:
    failure status "ci/circleci: test_docs_examples_ivy" is failing
    pending missing required status "ci/circleci: publish_snapshot"

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@kyliau

This comment has been minimized.

Copy link
Member Author

commented Jul 31, 2019

caretaker: test_docs_examples_ivy failure is unrelated to this change

@kyliau kyliau force-pushed the kyliau:getTemplateAt branch from 8f0210f to a7ae09a Aug 1, 2019

@alxhub alxhub closed this in e8b8f6d Aug 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.