-
Notifications
You must be signed in to change notification settings - Fork 393
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
@wire
computed properties should be restricted to values that cannot change
#3956
Comments
The minimal-change approach to this would be to perform static analysis to try to resolve values and determine whether or not they are primitives. However, a more radical approach might be to re-write how the computed properties are used by taking a snapshot. This would avoid the footgun of changing a variable that is used as a computed key, and it would also allow us to support any expression, not just identifiers and literals. An example transformation might look like this: const symbol = Symbol.for("key");
export default class Test extends LightningElement {
@wire(getFoo, {
[symbol]: "$prop1",
[Math.random()]: "$prop2",
notReactive: 'just hanging out'
})
wiredFoo;
} // output.js
const symbol = Symbol.for("key");
class Test extends LightningElement {
wiredFoo;
}
const keys = [symbol, Math.random()].map(String)
_registerDecorators(Test, {
wire: {
wiredFoo: {
adapter: getFoo,
dynamic: [symbol, "string"],
config: function ($cmp) {
return {
notReactive: 'just hanging out',
[keys[0]]: $cmp.prop1,
[keys[1]]: $cmp.prop2
};
}
}
}
}); |
Note this is technically a breaking change, but since this is a rarely-used API and we just recently unblocked it for non-strings, it's probably still safe to make this change without API versioning. |
I will try this one tomorrow. If I can't figure out where to change I will ask for help. |
I went to sleep 7 in the morning last night. Just woke up. So, because I need some rest, I decided to start it next monday. |
This issue has been linked to a new work item: W-15090483 |
Ok I have not had time to do it :( sorry |
I think we probably shouldn't call this "up for grabs" since I suspect it would be a breaking change. We should maybe do it, but we might have to do it behind API versioning or something. |
To enable reactivity, the
@wire
decorator transforms code in such a way that computed properties are re-computed whenever updates are needed. This means that variables used as property keys must have string representations that never change.Example transformation
The following snippets are excerpts, not complete code.For reactivity to properly function, the object returned by the
config
method must always have the keys in thedynamic
array.In #3955, we updated the
@wire
validation to only permit identifiers that were declared as aconst
. This prevents accidentally changing the value and breaking reactivity. However, this is not a complete solution, as the string representation of an object may change (e.g.{ toString: Math.random }
). Additionally, the solution does not permit constants that were imported from another file, even if those constants would otherwise be acceptable to use. (A simple workaround for the import problem is to declare a new constant in the component file.) We should validate that the value for a constant is not an object; only primitives should be allowed as computed property keys.Additionally, in the same PR we allowed primitives to be used directly as property keys, as that is perfectly valid JavaScript with a static string representation. We did not permit template literals, as they contain expressions that can change (e.g.
`${Math.random()}`
). We may want to do analysis of template literals, and allow them to be used if we can determine that they have a static string representation (e.g.`without expressions`
,`with ${1} static expression`
).For example, the following is currently allowed, but should not be.
And following examples are currently not allowed, but should be.
The text was updated successfully, but these errors were encountered: