Skip to content

fix(ivy): ensure unique template and listener function identifiers - #27766

Closed
gkalpak wants to merge 2 commits into
angular:masterfrom
gkalpak:fix-ivy-unique-tmpl-func-names
Closed

fix(ivy): ensure unique template and listener function identifiers#27766
gkalpak wants to merge 2 commits into
angular:masterfrom
gkalpak:fix-ivy-unique-tmpl-func-names

Conversation

@gkalpak

@gkalpak gkalpak commented Dec 19, 2018

Copy link
Copy Markdown
Member

PR Checklist

PR Type

  • Bugfix

What is the current behavior?

There can be identical template/listener function names for a component's template, if it has multiple similarly structured nested sub-templates or listeners.
This results in build errors: Identifier '<SOME_IDENTIFIER>' has already been declared

What is the new behavior?

This PR fixes this by ensuring that the template index is included in the contextName passed to the TemplateDefinitionBuilder responsible for processing nested sub-templates. Similarly, the template or element index is included in the listener names.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Blocking: #26947

@gkalpak gkalpak mentioned this pull request Dec 19, 2018
Closed
11 of 13 tasks
@ngbot ngbot Bot added this to the needsTriage milestone Dec 19, 2018
@mary-poppins

mary-poppins commented Dec 19, 2018

Copy link
Copy Markdown

You can preview f744618 at https://pr27766-f744618.ngbuilds.io/.

@gkalpak
gkalpak force-pushed the fix-ivy-unique-tmpl-func-names branch from f744618 to 6458bb7 Compare December 19, 2018 20:55
@mary-poppins

mary-poppins commented Dec 19, 2018

Copy link
Copy Markdown

You can preview 6458bb7 at https://pr27766-6458bb7.ngbuilds.io/.

petebacondarwin
petebacondarwin reviewed Dec 20, 2018
View reviewed changes
Comment thread packages/compiler-cli/test/compliance/r3_view_compiler_template_spec.ts Outdated

@petebacondarwin petebacondarwin Dec 20, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

unigue -> unique

@gkalpak gkalpak Dec 20, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

😱

petebacondarwin
petebacondarwin reviewed Dec 20, 2018
View reviewed changes
Comment thread packages/compiler-cli/test/compliance/r3_view_compiler_template_spec.ts Outdated

@petebacondarwin petebacondarwin Dec 20, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

unique

@gkalpak
gkalpak force-pushed the fix-ivy-unique-tmpl-func-names branch from 6458bb7 to 0c93201 Compare December 20, 2018 16:15
@mary-poppins

mary-poppins commented Dec 20, 2018

Copy link
Copy Markdown

You can preview 0c93201 at https://pr27766-0c93201.ngbuilds.io/.

@gkalpak
gkalpak force-pushed the fix-ivy-unique-tmpl-func-names branch from 0c93201 to fa94388 Compare December 21, 2018 09:17
@mary-poppins

mary-poppins commented Dec 21, 2018

Copy link
Copy Markdown

You can preview fa94388 at https://pr27766-fa94388.ngbuilds.io/.

@gkalpak
gkalpak force-pushed the fix-ivy-unique-tmpl-func-names branch from fa94388 to 47ad814 Compare December 21, 2018 09:25
@mary-poppins

mary-poppins commented Dec 21, 2018

Copy link
Copy Markdown

You can preview 47ad814 at https://pr27766-47ad814.ngbuilds.io/.

@gkalpak
gkalpak force-pushed the fix-ivy-unique-tmpl-func-names branch from 47ad814 to fd853ba Compare December 21, 2018 09:37
@mary-poppins

mary-poppins commented Dec 21, 2018

Copy link
Copy Markdown

You can preview fd853ba at https://pr27766-fd853ba.ngbuilds.io/.

@gkalpak
gkalpak force-pushed the fix-ivy-unique-tmpl-func-names branch from fd853ba to e864bf1 Compare December 21, 2018 10:23
@mary-poppins

mary-poppins commented Dec 21, 2018

Copy link
Copy Markdown

You can preview e864bf1 at https://pr27766-e864bf1.ngbuilds.io/.

@gkalpak
gkalpak force-pushed the fix-ivy-unique-tmpl-func-names branch from e864bf1 to 3964109 Compare December 21, 2018 10:50
@mary-poppins

mary-poppins commented Dec 21, 2018

Copy link
Copy Markdown

You can preview 3964109 at https://pr27766-3964109.ngbuilds.io/.

@gkalpak gkalpak added target: major This PR is targeted for the next major release action: review The PR is still awaiting reviews from at least one requested reviewer and removed PR target: TBD state: WIP labels Dec 21, 2018
kara
kara approved these changes Dec 21, 2018
View reviewed changes

@kara kara left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@kara kara removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Dec 21, 2018
@kara

kara commented Dec 21, 2018

Copy link
Copy Markdown
Contributor

presubmit

@kara kara added the action: merge The PR is ready for merge by the caretaker label Dec 21, 2018
@gkalpak
refactor(ivy): minor refactoring / code clean-up
be857ed
@gkalpak
fix(ivy): ensure unique template and listener function identifiers
f4f395d 
Previously, there could be identical template/listener function names
for a component's template, if it had multiple similarly structured
nested sub-templates or listeners.
This resulted in build errors:
`Identifier '<SOME_IDENTIFIER>' has already been declared`

This commit fixes this by ensuring that the template index is included
in the `contextName` passed to the `TemplateDefinitionBuilder`
responsible for processing nested sub-templates.
Similarly, the template or element index is included in the listener
names.
@gkalpak
gkalpak force-pushed the fix-ivy-unique-tmpl-func-names branch from 3964109 to f4f395d Compare December 26, 2018 20:05
@mary-poppins

mary-poppins commented Dec 26, 2018

Copy link
Copy Markdown

You can preview f4f395d at https://pr27766-f4f395d.ngbuilds.io/.

@gkalpak

gkalpak commented Dec 26, 2018

Copy link
Copy Markdown
Member Author

Rebased on master.

@kara kara closed this in d026b67 Jan 4, 2019
kara pushed a commit that referenced this pull request Jan 4, 2019
@gkalpak @kara
fix(ivy): ensure unique template and listener function identifiers (#…
48555f9 
…27766)

Previously, there could be identical template/listener function names
for a component's template, if it had multiple similarly structured
nested sub-templates or listeners.
This resulted in build errors:
`Identifier '<SOME_IDENTIFIER>' has already been declared`

This commit fixes this by ensuring that the template index is included
in the `contextName` passed to the `TemplateDefinitionBuilder`
responsible for processing nested sub-templates.
Similarly, the template or element index is included in the listener
names.

PR Close #27766
@gkalpak
gkalpak deleted the fix-ivy-unique-tmpl-func-names branch January 7, 2019 11:39
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Sep 14, 2019

Copy link
Copy Markdown

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 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@alxhub alxhub alxhub approved these changes
+2 more reviewers
@petebacondarwin petebacondarwin petebacondarwin approved these changes
@kara kara kara approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker cla: yes target: major This PR is targeted for the next major release type: bug/fix

Projects

None yet

Milestone

needsTriage

Development

Successfully merging this pull request may close these issues.

6 participants

@gkalpak @mary-poppins @kara @petebacondarwin @alxhub @googlebot