feat(babel-plugin-compiler): observe string, number, and identifier bracketed fields

[cardoso]

Details

Adds support for observing simple string and number bracketed fields (eg: ['with spaces'] and [1337]), as well as identifiers (eg: [symbol]). Support for non-computed numbers and strings is also included eg 1337 and 'with spaces'

String and number literals are very straightforward. Bracketed identifiers (to support symbol) deserve a deeper look.

Support for booleans [true] [false] is not included since it wasn't in the original issue, but it could be desirable for completeness and consistency, although it's hard for me to see why one would do that.

Closes #4547

Does this pull request introduce a breaking change?

• 💔 Yes, it does introduce a breaking change.

It can be a breaking change if a bracketed identifier, string or number property is expected to not be observable, but I doubt anyone would be relying on this behavior, especially since most linters will warn/error about it.

Does this pull request introduce an observable change?

• 🔬 Yes, it does include an observable change.

Simple bracketed identifier, strings and number properties will now be observed.

GUS work item

[cardoso]

I removed the observing of template literal computed fields, so this could be ready to merge if everything else is right.

[cardoso]

One thing that might be missing here is booleans [true] [false]. No idea why anyone would do that, but it could be included for the sake of completion and consistency.

[nolanlawson]

Thanks a lot for this PR! A few quick comments:

1. Since this is an observable change, I think we should put this behind API versioning. It is possible that existing components would be broken if they relied on these properties not being reactive. Also, it's a good carrot for folks to upgrade. This means adding a new feature to api-version.ts and, for the fixture tests, adding a config.json file with { "apiVersion": 63 } to enable the functionality.
2. I agree that booleans are a super-duper edge case and not worth supporting. Even numbers are kinda debatable – only "strings like this" and symbols are (I think) really obvious candidates for reactivity.
3. At runtime, we will probably want to restrict this to only expected types, so that people don't use computed properties as a backdoor to allowing every type under the sun. (Not sure how feasible it would be to restrict at the compiler level.)

@ekashida @jye-sf @wjhsf What do you think about supporting numbers?

[wjhsf]

What do you think about supporting numbers?

Any string, numeric, or symbol is a valid property key. If we support strings as property keys (computed and non-computed), then we should also support numeric literals.

I don't think we need to support boolean (and regex and null) literals in computed properties until somebody complains that they actually want to do that.

[nolanlawson]

The argument against numbers from my POV is mostly just that it "feels" weird, since it's typically only used in collections (e.g. arrays/HTMLCollections), and I can't imagine why you'd want to use that in an LWC component.

I could also potentially see some issues if we have a reactive property like 1337 which happens to conflict with the string property "1337". In ReactiveObserver we use a plain Object.create(null) object to track reactive properties, so we'd have to test how the keys might conflict:

lwc/packages/@lwc/engine-core/src/libs/mutation-tracker/index.ts

Line 62 in 13aa150

reactiveRecord[key as any] = reactiveObservers;

[wjhsf]

I could also potentially see some issues if we have a reactive property like 1337 which happens to conflict with the string property "1337".

Do we currently have handling for duplicate string/identifiers? { foo: 1, "foo": 2, foo: 3, "foo": 4 }

[nolanlawson]

It looks like for @tracked objects, we already conflate the number property 1337 and the string property "1337". Either are reactive.

I guess maybe this is fine because there's no difference in JavaScript between string keys and number keys in plain objects. I'm just wondering whether we really need to support this at the top-level in components. Even if you want your key to be [1337], it's a lot clearer to use ["1337"] since this is how JavaScript is going to treat it anyway.

[nolanlawson]

I thought about this PR a bit more. It occurred to me that we still don't support decorators (@api, @track, @wire) on these computed properties. So it's not really a full solution.

I feel like it might make more sense to just document this as a limitation of the framework, and then focus on fixing it once browsers have native support for decorators. At that point, it would be easy to support these decorators at the syntax level (since we wouldn't need our own Babel plugin), and we could potentially get rid of registerDecorators altogether which would resolve the reactivity issue.

[cardoso]

I'm closing this one for now then @nolanlawson . Feel free to reopen if things change.