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

ngTemplateOutlet modifies its initial context when getting a new context whose shape is the same #24515

Closed
divdavem opened this issue Jun 14, 2018 · 3 comments · Fixed by ngstack/translate#644
Assignees
Labels
area: common Issues related to APIs in the @angular/common package freq1: low P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: confirmed state: has PR type: bug/fix
Milestone

Comments

@divdavem
Copy link
Contributor

divdavem commented Jun 14, 2018

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question
[ ] Other... Please describe:

Current behavior

ngTemplateOutlet modifies the previous context when getting a new context whose shape is the same.

Expected behavior

ngTemplateOutlet should not change its context parameters in any case.

Minimal reproduction of the problem with instructions

https://stackblitz.com/edit/angular-ymixqh?file=src%2Fapp%2Fapp.component.ts

Click on "Swap". The contexts of the ngTemplateOutlets directives are swapped.
We should see "name = context 2" on the first line and "name = context 1" on the second line, but both lines show "name = context 2". The first ngTemplateOutlet has changed the name property of the first context when receiving the second context.

Note that the problem does not happen if the contexts do not have exactly the same set of properties (for example, when adding "foo: true" in the first context but not in the second, the behavior of ngTemplateOutlet is correct).

What is the motivation / use case for changing the behavior?

It is clearly a bug. It is confusing.

Environment


Angular version: 6.0.0

Browser: all

@divdavem divdavem changed the title ngTemplateOutletContext modifies its initial context when getting a new context whose shape is the same ngTemplateOutlet modifies its initial context when getting a new context whose shape is the same Jun 14, 2018
divdavem added a commit to divdavem/angular that referenced this issue Jun 14, 2018
@alexeagle alexeagle added the area: core Issues related to the framework runtime label Jun 14, 2018
@ngbot ngbot bot added this to the needsTriage milestone Jun 14, 2018
@mhevery
Copy link
Contributor

mhevery commented Jun 14, 2018

@ngbot ngbot bot modified the milestones: needsTriage, Backlog Jun 14, 2018
@pkozlowski-opensource pkozlowski-opensource added area: common Issues related to APIs in the @angular/common package and removed area: core Issues related to the framework runtime labels Jun 24, 2018
@waterplea
Copy link
Contributor

waterplea commented Aug 23, 2019

I got my static constants with some never changing content mutated by this and whole app got messed up by just using ngTemplateOutlet, built in trusted directive. Cannot believe this got no traction for a year, please do something about it. I understand the idea of optimizing it so it doesn't recreate the view, but this could be done much more safely.

@jelbourn jelbourn added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent and removed severity3: broken labels Oct 1, 2020
@crisbeto crisbeto self-assigned this Jan 8, 2021
crisbeto added a commit to crisbeto/angular that referenced this issue Jan 8, 2021
Currently `EmbeddedViewRef.context` is read-only which means that the only way to update
it is to mutate the object which can lead to some undesirable outcomes if the template
and the context are provided by an external consumer (see angular#24515).

These changes make the property writeable since there doesn't appear to be a specific
reason why it was readonly to begin with.
crisbeto added a commit to crisbeto/angular that referenced this issue Jan 8, 2021
Currently `NgTemplateOutlet` recreates its view if its template is swapped out or a context
object with a different shape is passed in. If an object with the same shape is passed in,
we preserve the old view and we mutate the previous object. This mutation of the original
object can be undesirable if two objects with the same shape are swapped between two
different template outlets.

The current behavior is a result of a limitation in `core` where the `context` of an embedded
view is read-only, however a previous commit made it writeable.

These changes resolve the context mutation issue and clean up a bunch of unnecessary
logic from `NgTemplateOutlet` by taking advantage of the earlier change.

Fixes angular#24515.
crisbeto added a commit to crisbeto/angular that referenced this issue Jan 8, 2021
Currently `NgTemplateOutlet` recreates its view if its template is swapped out or a context
object with a different shape is passed in. If an object with the same shape is passed in,
we preserve the old view and we mutate the previous object. This mutation of the original
object can be undesirable if two objects with the same shape are swapped between two
different template outlets.

The current behavior is a result of a limitation in `core` where the `context` of an embedded
view is read-only, however a previous commit made it writeable.

These changes resolve the context mutation issue and clean up a bunch of unnecessary
logic from `NgTemplateOutlet` by taking advantage of the earlier change.

Fixes angular#24515.
crisbeto added a commit to crisbeto/angular that referenced this issue Jan 23, 2021
Currently `EmbeddedViewRef.context` is read-only which means that the only way to update
it is to mutate the object which can lead to some undesirable outcomes if the template
and the context are provided by an external consumer (see angular#24515).

These changes make the property writeable since there doesn't appear to be a specific
reason why it was readonly to begin with.
crisbeto added a commit to crisbeto/angular that referenced this issue Jan 23, 2021
Currently `NgTemplateOutlet` recreates its view if its template is swapped out or a context
object with a different shape is passed in. If an object with the same shape is passed in,
we preserve the old view and we mutate the previous object. This mutation of the original
object can be undesirable if two objects with the same shape are swapped between two
different template outlets.

The current behavior is a result of a limitation in `core` where the `context` of an embedded
view is read-only, however a previous commit made it writeable.

These changes resolve the context mutation issue and clean up a bunch of unnecessary
logic from `NgTemplateOutlet` by taking advantage of the earlier change.

Fixes angular#24515.
crisbeto added a commit to crisbeto/angular that referenced this issue Jan 23, 2021
Currently `NgTemplateOutlet` recreates its view if its template is swapped out or a context
object with a different shape is passed in. If an object with the same shape is passed in,
we preserve the old view and we mutate the previous object. This mutation of the original
object can be undesirable if two objects with the same shape are swapped between two
different template outlets.

The current behavior is a result of a limitation in `core` where the `context` of an embedded
view is read-only, however a previous commit made it writeable.

These changes resolve the context mutation issue and clean up a bunch of unnecessary
logic from `NgTemplateOutlet` by taking advantage of the earlier change.

Fixes angular#24515.
crisbeto added a commit to crisbeto/angular that referenced this issue Feb 18, 2021
Currently `EmbeddedViewRef.context` is read-only which means that the only way to update
it is to mutate the object which can lead to some undesirable outcomes if the template
and the context are provided by an external consumer (see angular#24515).

These changes make the property writeable since there doesn't appear to be a specific
reason why it was readonly to begin with.
crisbeto added a commit to crisbeto/angular that referenced this issue Feb 18, 2021
Currently `NgTemplateOutlet` recreates its view if its template is swapped out or a context
object with a different shape is passed in. If an object with the same shape is passed in,
we preserve the old view and we mutate the previous object. This mutation of the original
object can be undesirable if two objects with the same shape are swapped between two
different template outlets.

The current behavior is a result of a limitation in `core` where the `context` of an embedded
view is read-only, however a previous commit made it writeable.

These changes resolve the context mutation issue and clean up a bunch of unnecessary
logic from `NgTemplateOutlet` by taking advantage of the earlier change.

Fixes angular#24515.
@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 Mar 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.