`@wire` computed properties should be restricted to values that cannot change

[wjhsf]

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. ```js // input.js const symbol = Symbol.for("key"); export default class Test extends LightningElement { @wire(getFoo, { [symbol]: "$prop1", string: "$prop2", notReactive: 'just hanging out' }) wiredFoo; } ``` ```js // output.js const symbol = Symbol.for("key"); class Test extends LightningElement { wiredFoo; } _registerDecorators(Test, { wire: { wiredFoo: { adapter: getFoo, dynamic: [symbol, "string"], config: function ($cmp) { return { notReactive: 'just hanging out', [symbol]: $cmp.prop1, string: $cmp.prop2 }; } } } }); ``` For reactivity to properly function, the object returned by the `config` method must always have the keys in the `dynamic` array.

In #3955, we updated the @wire validation to only permit identifiers that were declared as a const. 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.

// every time the property is computed, it results in a different value
const toStringChanges = { toString: Math.random }
@wire(getFoo, { [toStringChanges]: "$prop" }) nonDeterministicObject;

And following examples are currently not allowed, but should be.

// imported constants should be treated the same as constants declared in the file
import { IMPORTED_CONSTANT } from "./config";
@wire(getFoo, { [IMPORTED_CONSTANT]: "$prop" }) importedConstant;

// the computed string will never change for these template literals
@wire(getFoo, [`no expressions`]: "$prop" }) noExpressions;
@wire(getFoo, [`${1} expression`]: "$prop" }) oneExpression;

[wjhsf]

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
        };
      }
    }
  }
});

[nolanlawson]

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.

[AllanOricil]

I will try this one tomorrow. If I can't figure out where to change I will ask for help.

[AllanOricil]

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.

[git2gus[bot]]

This issue has been linked to a new work item: W-15090483

[AllanOricil]

Ok I have not had time to do it :( sorry I will let you do it. I can review pr after if u need some help there. Sorry again 😅

[nolanlawson]

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.