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): report better error on interpolation in an expression #30300

Closed
wants to merge 1 commit into from
Closed

fix(compiler): report better error on interpolation in an expression #30300

wants to merge 1 commit into from

Conversation

krzysztof-grzybek
Copy link
Contributor

@krzysztof-grzybek krzysztof-grzybek commented May 7, 2019 •
edited by JoostK

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?

Fixes #27740

What is the new behavior?

Compiler report proper "unexpected token" error.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@krzysztof-grzybek krzysztof-grzybek requested a review from a team as a code owner May 7, 2019 15:53
@kara kara added the area: core Issues related to the framework runtime label May 7, 2019
@ngbot ngbot bot added this to the needsTriage milestone May 7, 2019
@JoostK JoostK added area: compiler Issues related to `ngc`, Angular's template compiler and removed area: core Issues related to the framework runtime labels Nov 8, 2020
@pullapprove pullapprove bot requested a review from JoostK November 8, 2020 12:52
@JoostK JoostK added type: bug/fix P4 A relatively minor issue that is not relevant to core functions labels Nov 8, 2020
@atscott
Copy link
Contributor

atscott commented Nov 12, 2020

Hi @krzysztof-grzybek, thanks for the fix. Are you still interested in working on this? You'll need to rebase this and update the test a bit, but other than that this looks like a reasonable fix to me.

@krzysztof-grzybek krzysztof-grzybek force-pushed the better-error-on-interpolation-in-expression branch 2 times, most recently from e4aa699 to 1839990 Compare November 15, 2020 18:49
@krzysztof-grzybek
Copy link
Contributor Author

Yes, I will fix it in the next few days, probably tommorow.

@krzysztof-grzybek krzysztof-grzybek force-pushed the better-error-on-interpolation-in-expression branch 3 times, most recently from 817b001 to d94d7b5 Compare November 23, 2020 20:29
@krzysztof-grzybek
fix(compiler): report better error on interpolation in an expression
6a485bd 
Compiler results in weird error message when encounters interpolation inside
existing expression context, e.g. *ngIf="name {{ name }}"
@krzysztof-grzybek krzysztof-grzybek force-pushed the better-error-on-interpolation-in-expression branch from d94d7b5 to 6a485bd Compare November 24, 2020 18:25
@krzysztof-grzybek
Copy link
Contributor Author

@atscott I updated my PR, I think it's ready for code review.

@petebacondarwin petebacondarwin added action: presubmit The PR is in need of a google3 presubmit action: review The PR is still awaiting reviews from at least one requested reviewer labels Nov 25, 2020
petebacondarwin
petebacondarwin approved these changes Nov 25, 2020
View reviewed changes
Copy link
Member

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

LGTM!

@petebacondarwin petebacondarwin added the target: patch This PR is targeted for the next patch release label Nov 25, 2020
@atscott
Copy link
Contributor

atscott commented Nov 25, 2020

presubmit

@atscott atscott removed the action: presubmit The PR is in need of a google3 presubmit label Nov 25, 2020
JoostK
JoostK approved these changes Nov 25, 2020
View reviewed changes
Copy link
Member

@JoostK JoostK left a comment

Choose a reason for hiding this comment

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

The change LGTM. In the future we may want to look into calling _checkNoInterpolation in Parser.parseTemplateBindings, similar to what is done for regular bindings, but that might have unforeseen consequences on existing code so let's get this change in.

@atscott atscott added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Nov 25, 2020
jessicajaniuk pushed a commit that referenced this pull request Nov 25, 2020
@krzysztof-grzybek @thePunderWoman
fix(compiler): report better error on interpolation in an expression (#…
6dc74fd 
…30300)

Compiler results in weird error message when encounters interpolation inside
existing expression context, e.g. *ngIf="name {{ name }}"

PR Close #30300
@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 Dec 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@petebacondarwin petebacondarwin petebacondarwin approved these changes

@JoostK JoostK JoostK approved these changes

@atscott atscott atscott approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler cla: yes P4 A relatively minor issue that is not relevant to core functions target: patch This PR is targeted for the next patch release type: bug/fix
Projects
None yet
Milestone
needsTriage
Development

Successfully merging this pull request may close these issues.

Using {{}} interpolation in an expression causes toUpperCase error
6 participants
@krzysztof-grzybek @atscott @petebacondarwin @JoostK @kara @googlebot