-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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(ivy): incorrectly generating shared pure function between null and object literal #35481
Conversation
2e2cdd1
to
5c54e91
Compare
@Component({ | ||
template: ` | ||
<div [dir]="{foo: null}"></div> | ||
<div [dir]="{foo: getFoo()}"></div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this one has been an issue even before #33705, but we just never hit it.
db86429
to
f95ca65
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the commit message as well - it's now out of date since the changes we made yesterday :-)
…d object literal In angular#33705 we made it so that we generate pure functions for object/array literals in order to avoid having them be shared across elements/views. The problem this introduced is that further down the line the `ContantPool` uses the generated literal in order to figure out whether to share an existing factory or to create a new one. `ConstantPool` determines whether to share a factory by creating a key from the AST node and using it to look it up in the factory cache, however the key generation function didn't handle function invocations and replaced them with `null`. This means that the key for `{foo: pureFunction0(...)}` and `{foo: null}` are the same. These changes rework the logic so that instead of generating a `null` key for function invocations, we generate a variable called `<unknown>` which shouldn't be able to collide with anything. Fixes angular#35298.
…null and object literal
f95ca65
to
b87b90f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for the detailed commit message! :)
…d object literal (#35481) In #33705 we made it so that we generate pure functions for object/array literals in order to avoid having them be shared across elements/views. The problem this introduced is that further down the line the `ContantPool` uses the generated literal in order to figure out whether to share an existing factory or to create a new one. `ConstantPool` determines whether to share a factory by creating a key from the AST node and using it to look it up in the factory cache, however the key generation function didn't handle function invocations and replaced them with `null`. This means that the key for `{foo: pureFunction0(...)}` and `{foo: null}` are the same. These changes rework the logic so that instead of generating a `null` key for function invocations, we generate a variable called `<unknown>` which shouldn't be able to collide with anything. Fixes #35298. PR Close #35481
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
In #33705 we made it so that we generate pure functions for object/array literals in order to avoid having them be shared across elements/views. The problem this introduced is that further down the line the
ContantPool
uses the generated literal in order to figure out whether to share an existing factory or to create a new one.ConstantPool
determines whether to share a factory by creating a key from the AST node and using it to look it up in the factory cache, however the key generation function didn't handle function invocations and replaced them withnull
. This means that the key for{foo: pureFunction0(...)}
and{foo: null}
are the same.These changes rework the logic so that instead of generating a
null
key for function invocations, we generate a variable called<unknown>
which shouldn't be able to collide with anything.Fixes #35298.