`@wire()` protocol reform RFC #14

caridy · 2019-08-20T21:20:12Z

Rendered

cc: @jbenallen @kevinv11n @davidturissini @diervo

text/0000-wire-reform.md

pmdartus · 2019-08-21T13:16:30Z

text/0000-wire-reform.md

+* `@wire` decorator will create a mutation tracking phase to track any access executed during the computation of the config before calling the `update` routine to be able to detect mutations on those values and issue another update on the adapter instance.
+* `@wire` decorator will implement the logic to provide contextual information when requested by the wire adapter.
+* `@lwc/compiler` will provide a config function per `@wire()` declaration to produce a new config object when invoked with the `component` as the first argument. The `@wire` adapter can rely on that config function to produce a new config object at will.
+* `@lwc/wire-service` becomes lightning platform specific for most part (`register()` method), since anyone can implement the wire adapter protocol.


Since this proposal drops the wire adapter registration, is the @lwc/wire-service package will only be used for legacy wires. Once all the wire adapters have been updated we should get rid of this package.

YES! I believe that package will become just a referral implementation, an optional base implementation, and a bunch of types.

pmdartus · 2019-08-21T13:23:13Z

text/0000-wire-reform.md

+
+# How we teach this
+
+* For adapter consumers, nothing changes.


Side note: there may be some changes that we can do to the @salesforce/wire-service-jest-util project to make wire adapter mocking more pleasant.

pmdartus · 2019-08-21T13:26:34Z

text/0000-wire-reform.md

+
+# Unresolved questions
+
+* In the current implementation, a wired field is `writable`, which means the component author can alter the value of the field at will. What should we do? a) throw on setter, b) do nothing on setter, c) preserve the current semantics. This is a breaking change if we do a) or b), while the current behavior is weird.


While the semantic can be improved with a read-only field, I don't see any major issue with the current behavior.
At worst developers can override a property that is set by the @wire decorator, which is not harmful IMO.

Why don't we do (a) in dev mode akin to the behavior of @api props?

pmdartus · 2019-08-21T13:27:49Z

text/0000-wire-reform.md

+# Unresolved questions
+
+* In the current implementation, a wired field is `writable`, which means the component author can alter the value of the field at will. What should we do? a) throw on setter, b) do nothing on setter, c) preserve the current semantics. This is a breaking change if we do a) or b), while the current behavior is weird.
+* How context providers can be provisioned? In theory, a context provider is bound to a particular framework/system, while the context consumer is abstracted out in the wire adapter, and specific implementations per framework can provide the piping into the wire adapter protocol via the second argument in the constructor and the `adapter.context(uid, value)` mechanism. Is this sufficient?


I am not sure to understand, what do you mean here?

let's chat about this today.

The context aspect of this proposal isn't detailed. I suggest removing it until we sort out those details. I don't believe its absence invalidates nor decreases the value of the rest of the proposal.

Co-Authored-By: Pierre-Marie Dartus <p.dartus@salesforce.com>

text/0000-wire-reform.md

ravijayaramappa · 2019-08-21T18:10:32Z

text/0000-wire-reform.md

+* To create an instance of an adapter and link it to the host.
+* To signal to the adapter instance when the component is connected or disconnected.
+* To signal to the adapter instance when the config have changed by providing the new config object.
+* To extract the config value from the host object by relying on the compiler's wire metadata.


host object or host element?
I guess you want to call it an object because we want to use it with other things, not just components.

well, the wire decorator is LWC specific... in which case this is the component reference. I will update the sentence to reflect that. I will also add the LWC wire decorator to be explicit about what decorator are we talking about.

ravijayaramappa · 2019-08-21T18:12:15Z

text/0000-wire-reform.md

+* To signal to the adapter instance when the component is connected or disconnected.
+* To signal to the adapter instance when the config have changed by providing the new config object.
+* To extract the config value from the host object by relying on the compiler's wire metadata.
+* To signal to the adapter instance when a context is available by providing the new context value. 


How is this point different from responsibility # 3?

3 is about connect and disconnect, this is about contextual configuration.

text/0000-wire-reform.md

pmdartus · 2019-08-22T09:33:07Z

text/0000-wire-reform.md

+}
+
+interface WireAdapterConstructor {
+    new (callback: EmitDataCallback, contextualizer?: RequestContextCallback): WireAdapter;


In order to accept more parameters in the future, I would suggest passing an object instead of 2 callbacks.

interface WireAdapterConstructorConfig { callback: EmitDataCallback, contextualizer?: RequestContextCallback } interface WireAdapterConstructor { new (config: WireAdapterConstructorConfig): WireAdapter; }

I will take care of that.

Co-Authored-By: Ravi Jayaramappa <ravi.jayaramappa@salesforce.com>

text/0000-wire-reform.md

kevinv11n · 2019-08-23T18:38:50Z

text/0000-wire-reform.md

+
+There is a dual-dependency between `@lwc/engine` and `@lwc/wire-service`, even though neither of those two packages are importing each other, it is the responsibility of the adapter author to connect them via `registerWireService(register)` where `registerWireService()` is provided by `@lwc/wire-service` and `register()` is provided by `@lwc/engine`. This is, by itself, complex and confusing. Additionally, there is another `register()` method from `@lwc/wire-service` that is used by authors to link their wire adapters with their adapter ID (identity). This process also posses a limitation, and unnecessary dependency, making adapters to tied to LWC.
+
+Additionally, there are various situations where the wired field or method is not behaving correctly, the most notable example is when a configuration value that uses a member expression might not trigger the update on the config. (e.g. `wire(foo, { x: '$foo.bar' }) data`, if `foo.bar` changes, config is not updated). This is because the wire decorator is not relying on the reactivity system used by LWC, and instead it relies on getter and setters that are slower, intrusive, complex and do not cover the whole spectrum of mutations that could occur on a component.


e.g. wire(foo, { x: '$foo.bar' }) data, if foo.bar changes, config is not updated

This is a limitation we designed into the system. What you're proposing is an enhancement which I fully support.

This is because the wire decorator is not relying on the reactivity system used by LWC

In the paragraph above you assert the need to decouple the wire service from LWC yet here you are taking issue that the wire service doesn't use LWC.

I agree with the concept but the phrasing needs work.

text/0000-wire-reform.md

kevinv11n · 2019-08-23T18:56:02Z

text/0000-wire-reform.md

+
+# Unresolved questions
+
+* In the current implementation, a wired field is `writable`, which means the component author can alter the value of the field at will. What should we do? a) throw on setter, b) do nothing on setter, c) preserve the current semantics. This is a breaking change if we do a) or b), while the current behavior is weird.


Why don't we do (a) in dev mode akin to the behavior of @api props?

kevinv11n · 2019-08-23T18:57:06Z

text/0000-wire-reform.md

+# Unresolved questions
+
+* In the current implementation, a wired field is `writable`, which means the component author can alter the value of the field at will. What should we do? a) throw on setter, b) do nothing on setter, c) preserve the current semantics. This is a breaking change if we do a) or b), while the current behavior is weird.
+* How context providers can be provisioned? In theory, a context provider is bound to a particular framework/system, while the context consumer is abstracted out in the wire adapter, and specific implementations per framework can provide the piping into the wire adapter protocol via the second argument in the constructor and the `adapter.context(uid, value)` mechanism. Is this sufficient?


The context aspect of this proposal isn't detailed. I suggest removing it until we sort out those details. I don't believe its absence invalidates nor decreases the value of the rest of the proposal.

Co-Authored-By: Kevin Venkiteswaran <kvenkiteswaran@salesforce.com>

…ire-reform

pmdartus · 2019-08-28T14:13:02Z

text/0000-wire-reform.md

+_Notes:_
+
+* Not all environments will support or need context (e.g.: preloading LDS data), but does supporting it can rely on the static field called `contextSchema` to provide the context value when available.
+* Some environments might choose to implement validation rules for `configSchema` and `contextSchema` to guarantee compliance.


What is the advantage configSchema and contextSchema vs doing the validation by hand in the update callback?
It feels that we are adding higher-level abstractions to low-level APIs.

we need to know, without invoking update, if the adapter needs context... to try to provision it.

text/0000-wire-reform.md

Co-Authored-By: Jose David Rodriguez Velasco <jodarove@gmail.com>

text/0000-wire-reform.md

kevinv11n · 2019-08-28T21:00:45Z

text/0000-wire-reform.md

+
+_Notes:_
+
+* Not all environments will support or need context (e.g.: preloading LDS data), but does supporting it can rely on the static field called `contextSchema` to provide the context value when available.


I don't understand this sentence. Can you please clarify?

trying to say that the runtime is responsible for providing or not context support, some will, some will not. Does that support context can do so by inspecting the wire adapter's context schema on the individual basis.

text/0000-wire-reform.md

kevinv11n · 2019-08-28T21:49:47Z

text/0000-wire-reform.md

+
+* `$token` can only appear as the value of a top level member property, e.g.: `@wire(foo, { x: '$prop1' })` will continue to be valid, while `@wire(foo, { x: { y: '$prop1' } })` will throw a compiler error, while today it doesn't but the value is never transformed, and remains as a string value. This is a non-backwards compatible change, but we believe this is a very low risk for something that was not working as expected.
+* there will be no identity for inline JSON objects when assigned to a property in the config object, e.g.: `@wire(foo, { x: { y: 1 } })` where the value of `x` will be computed every time, instead of cached per instance or per class as today.
+* there will be identity preserved when assigning a reference values in the config object, e.g.: `@wire(foo, { x: someValue })` where the value of `x` will be a reference to `someValue` during the class declaration.


I think you mean that someValue retains identity. Can you confirm and clarify?

yes, someValue retains identity, and x points to it.

additionally:
a reference values => reference values

kevinv11n · 2019-08-28T21:50:11Z

text/0000-wire-reform.md

+* `$token` can only appear as the value of a top level member property, e.g.: `@wire(foo, { x: '$prop1' })` will continue to be valid, while `@wire(foo, { x: { y: '$prop1' } })` will throw a compiler error, while today it doesn't but the value is never transformed, and remains as a string value. This is a non-backwards compatible change, but we believe this is a very low risk for something that was not working as expected.
+* there will be no identity for inline JSON objects when assigned to a property in the config object, e.g.: `@wire(foo, { x: { y: 1 } })` where the value of `x` will be computed every time, instead of cached per instance or per class as today.
+* there will be identity preserved when assigning a reference values in the config object, e.g.: `@wire(foo, { x: someValue })` where the value of `x` will be a reference to `someValue` during the class declaration.
+* every time that `adapter.update()` is invoked, a new config object will be provided as a first argument, no identity is preserved in this case.


Do you mean that the config object's identity is not preserved whereas the values inside of it that are references will retain identity?

Consider this example:

@wire(foo, { x: { y: 1, z: someValue }, a: someOtherValue })

If update(config) is invoked twice, where first is the argument for first invocation and second is second invocation:

first.x === second.x; // false first.x.someValue === second.x.someValue; // true first.a === second.a; // true

Also want to confirm that wire adapters should still treat the config as immutable regardless of whether it is at runtime

yes, all your assumptions are correct. maybe we should clarify that... with your example or something.

additionally, first !== second

text/0000-wire-reform.md

Co-Authored-By: Kevin Venkiteswaran <kvenkiteswaran@salesforce.com>

mburr-salesforce · 2019-09-04T18:11:50Z

text/0000-wire-reform.md

+
+```ts
+interface WireAdapter {
+    update(config: ConfigValue, context?: ContextValue);


Do you have an example of the difference between config and context? I'm trying to understand what would motivate an adapter to request/accept information via one vs the other.

another good question, I myself struggled with this, I wanted to make them one thing, but @diervo and @jbenallen convinced me that these should remain separate, one of them optional, mostly because of the types... and validations that we can do in the future, and how easy it is for the adapter to understand the channel in which those two set of data are getting collected.

A good example is probably an adapter that load records, where the record indicator is contextual (the tab that you are on) and the number of elements is configurable.

@caridy @diervo It looks like the RFC does not match the implementation:
update(config: Record<string, any>)
Thus, would be great to update the RFC as it somehow serves as the documentation.

mburr-salesforce · 2019-09-04T18:15:36Z

text/0000-wire-reform.md

+
+```ts
+interface WireAdapter {
+    update(config: ConfigValue, context?: ContextValue);


The current implementation requires wire adapters to emit null when presented with incomplete configs. Does this new protocol guarantee that update is only ever invoked with fully-resolved configs (which implies that an incomplete config is an error on the component author's part)?

This is a very good question. We have a more flexible API now. The implementation in @lwc/wire-service abstraction does take care of the null value before dispatching the config event IIRC (I will double check), but the wire protocol itself doesn't care about that anymore, that's for the adapters to decide when to go and process the new config or the new context.

I believe the null handling was internal to LDS. I don't see such handling in https://github.com/salesforce/lwc/blob/master/packages/@lwc/wire-service/src/index.ts#L113-L117

IIRC it might have been a side effect of the fact that the wire adapters were hooked up before the $ references could be expanded? The adapters had to specifically not emit an error in those cases or they would (at least in lds220) never be able to emit data afterwards.

thanks @kevinv11n, that's what I thought! we are good!

mburr-salesforce · 2019-09-04T18:18:14Z

text/0000-wire-reform.md

+    configSchema?: Record<string, WireAdapterSchemaValue>;
+    contextSchema?: Record<string, WireAdapterSchemaValue>;
+}
+type DataCallback = (value: any) => void;


Is value here a { data: ..., error: ... } construct? If not, how does the adapter indicate errors?

The protocol is not enforcing such structure. We have debated that in other forums multiple times... we ended up letting individual adapters to implement their own protocol when it comes to data and error. We do offer that as a guideline... but there are cases in which that makes no sense... (e.g.: navigation context). This method in this example is precisely to highlight that, you can make it { data: value } it you wish.

caridy · 2019-09-10T20:11:58Z

thanks everyone for the review, we are merging this today and start focusing on the implementation on PR salesforce/lwc#1459

priandsf · 2019-12-06T16:45:43Z

text/0000-wire-reform.md

+
+There exist a few restrictions and ambiguities with the IDL for the config object in `@wire` decorator declarations. This section will describe the changed semantics. Most use cases of `@wire` are unaffected. 
+
+* `$token` can only appear as the value of a top level member property, e.g.: `@wire(foo, { x: '$prop1' })` will continue to be valid, while `@wire(foo, { x: { y: '$prop1' } })` will throw a compiler error, while today it doesn't but the value is never transformed, and remains as a string value. This is a non-backwards compatible change, but we believe this is a very low risk for something that was not working as expected.


@caridy I faced the need of having a $token injected into a non top level property. Is there a technical limitation why this can't be done, or is it because of a potential performance concern with object comparison?
On the same subject, I also faced the need of having the $token being replaced as part of a string, like in:

@track offset = 0 @track limit = 5 @wire(useFetch, { url: '/users?offset=$offset&limit=$limit', ...

Or, even better, I would love some of these parameters to be functions like the peudo code bellow:

@track offset = 0 @track limit = 5 @wire(useFetch, { url: url('/users', {offset: '$offset', limit: '$limit'} ), ...

It probably means that the whole json object should be recreated on any change, and compared to the previous instance. As there are generally few properties, this should not be a performance problem.

* wire reform first draft * Apply suggestions from code review Co-Authored-By: Pierre-Marie Dartus <p.dartus@salesforce.com> * Apply suggestions from code review Co-Authored-By: Ravi Jayaramappa <ravi.jayaramappa@salesforce.com> * Apply suggestions from code review Co-Authored-By: Kevin Venkiteswaran <kvenkiteswaran@salesforce.com> * update on the wire-reform RFC * wire reform: adding ContextProvider * Update text/0000-wire-reform.md Co-Authored-By: Jose David Rodriguez Velasco <jodarove@gmail.com> * more notes about backwards compatibility * Apply suggestions from code review Co-Authored-By: Kevin Venkiteswaran <kvenkiteswaran@salesforce.com>

wire reform first draft

1b30d53

pmdartus reviewed Aug 21, 2019

View reviewed changes

Apply suggestions from code review

1b433ad

Co-Authored-By: Pierre-Marie Dartus <p.dartus@salesforce.com>

caridy mentioned this pull request Aug 21, 2019

refactor(wire-service): wire decorator reform salesforce/lwc#1459

Merged

ravijayaramappa reviewed Aug 21, 2019

View reviewed changes

text/0000-wire-reform.md Outdated Show resolved Hide resolved