proposal(engine): RFC for wire adapter #74
Conversation
Benchmark comparisonBase commit:
|
docs/proposals/0103-wire-adapters.md
Outdated
| * Caching mechanism is still controlled by the platform via the `@wire` decorator. | ||
| * Consumers of the wire adapter via `@wire` decorator can rely on a very simple data structure. | ||
| * It is possible to consume the adapter directly by just invoking the callable. | ||
| * Nop registration needed for adapters, which means we can simply control the access by using `import` static analysis. |
There was a problem hiding this comment.
This proposal transfers the responsibility of adapter discovery and registration from wire service to the compiler, what if the import doesn't exist until runtime? A typical example would be importing an Apex method. I feel we still need something that allows us to add adapters ad-hoc. Also a side question, in the ideal scenario of this proposal, wire service almost feels redundant, you just need import, @wire decorator, compiler, and maybe component constructor to setup the data reactivity?
There was a problem hiding this comment.
This proposal transfers the responsibility of adapter discovery and registration from wire service to the compiler, what if the import doesn't exist until runtime?
I don't fully understand this. But yes, certainly no discovery is needed. As for the compiler, no compilation is neither (only metadata extraction is required, but that happens for all decorators anyway).
A typical example would be importing an Apex method.
Not sure what you mean, can you elaborate more?
wire service almost feels redundant
YES! that's the intent. The @wire decorator can do all that as internal implementation detail, not need to have a separate public entity that registers and manage adapters.
docs/proposals/0103-wire-adapters.md
Outdated
|
|
||
| #### Cons | ||
|
|
||
| * Adapter doesn't really have control, it doesn't know the caller, and it can't do much. |
There was a problem hiding this comment.
I think this is even true with current wire service architecture. Adapters do not know their callers nor the context.
docs/proposals/0103-wire-adapters.md
Outdated
|
|
||
| * should the config object be a simple `Record<string, string | number>`, or could it be a complex/multilevel object? | ||
| * should `this.foo.data` be "reactive" only if the adapter choose to? or should it always be reactive? | ||
| * should `this.foo.data` be "read only" only if the adapter choose to? or should it always be read only? |
There was a problem hiding this comment.
if it's not read-only then I want to understand the data flow implication
There was a problem hiding this comment.
that's why I put this in the open questions. I'm ok with making this an invariant.
docs/proposals/0103-wire-adapters.md
Outdated
| * additional details can be added to `this.foo` by expanding the `@wire` decorator protocol. e.g.: "inflight" flag or `refresh()` mechanism. | ||
| * `this.foo` object should always be reactive. | ||
| * `this.foo.data` will be read only by the `@wire` decorator protocol. | ||
| * `this.foo.data` will be cached by the `@wire` decorator protocol by using the `config` as its key. |
There was a problem hiding this comment.
@kevinv11n how does LDS caching work? do you see that become the norm for wire adapters? or is that specific to LDS adapters?
There was a problem hiding this comment.
I envisioned that caching can be implemented at @wire and/or LDS abstraction layer (APIs) that will be used by some adapters that need to talk to LDS.
docs/proposals/0103-wire-adapters.md
Outdated
| * `foo` property will always be an object with a well defined API specified by the `@wire` decorator. | ||
| * `@wire` decorator is responsible for invoking the adapter (when needed) with the computed configuration. (in this case `'$id'` will be replaced with the value of `this.id` before invoking the adapter). | ||
| * the result of calling the adapter must be a promise, and depending on its resolution, `foo.data` and `foo.error` will be allocated accordingly. | ||
| * additional details can be added to `this.foo` by expanding the `@wire` decorator protocol. e.g.: "inflight" flag or `refresh()` mechanism. |
There was a problem hiding this comment.
seems contradict to Kevin's proposal that refresh is a capability of the adapter and it requires explicit import,
could you elaborate more on what this means?
expanding the
@wiredecorator protocol
There was a problem hiding this comment.
I don't remember the outcome of our last conversation! if we settle on refresh be imported, then we are good! @kevinv11n can you confirm? On the other hand, inflight is just a tied to the promise lifecycle, and can be implemented by the @wire decorator.
docs/proposals/0103-wire-adapters.md
Outdated
| * The adapter must be callable that expects exactly one argument. | ||
| * The argument provided to the adapter will be the *resolved* value specified as the second argument of the `@wire` decorator. We call this the adapter configuration, and its type could be anything. | ||
| * The adapter configuration is part of the public API specified per adapter. | ||
| * The result of calling the adapter must be a promise. |
There was a problem hiding this comment.
I'm aiming for the simple one! And a promise seems to be the lower-common denominator. But I'm ok extending this to observables as part of the extended protocol of the adapters.
There was a problem hiding this comment.
I have added the proposal 2 to focus on observables
docs/proposals/0103-wire-adapters.md
Outdated
|
|
||
| #### Defining a Wire Adapter | ||
|
|
||
| Writing a new wire adapter should be as easy as implementing a callback with a simple protocol: |
There was a problem hiding this comment.
i guess nothing prevents me from using the adapter w/o @wire? so if I were to create my own caching with importing the adapter as the fetch mechanism, it would be a functional scenario, do we want to do anything to discourage that?
There was a problem hiding this comment.
That's in fact the intent. People should be able to create their own adapters to do whatever they feel is right. We just need to find the right balance on how to restrict the behavior of such adapters to avoid the common pitfalls for these use-cases.
Benchmark comparisonBase commit:
|
Benchmark comparisonBase commit:
|
docs/proposals/0103-wire-adapters.md
Outdated
|
|
||
| * `foo` property will always be an object with a well defined API specified by the `@wire` decorator. | ||
| * `@wire` decorator is responsible for invoking the adapter (when needed) with the computed configuration. (in this case `'$id'` will be replaced with the value of `this.id` before invoking the adapter). | ||
| * the result of calling the adapter must be a promise, and depending on its resolution, `foo.data` and `foo.error` will be allocated accordingly. |
There was a problem hiding this comment.
contradicts to the above section
The result of calling the adapter must be an observable object.
There was a problem hiding this comment.
good catch, will fix it.
docs/proposals/0103-wire-adapters.md
Outdated
| #### Cons | ||
|
|
||
| * A fundamental problem with this proposal is that it does not provide a way for the `@wire` decorator to update the data unless the config changes due to a mutation in the component's instance state or properties. | ||
| * Adapter doesn't really have control, it doesn't know the caller, and it can't do much. |
There was a problem hiding this comment.
If wire adapters are limited to only having access to that config object, they can't be used to inject a value which depends on the caller. That means it can't be used to inject context data that is provided by some ancestor. That may be fine, but a different mechanism would be needed instead of @wire for that use case.
There was a problem hiding this comment.
you might be right! this might break some of the assumptions used by the Context's proposal.
There was a problem hiding this comment.
The latest commit doesn't address this comment. Are we separating access to context data for wire adapters from this proposal?
docs/proposals/0103-wire-adapters.md
Outdated
|
|
||
| ### Proposal 1: Simple Promise Based Callbacks | ||
|
|
||
| * wire adapters must be explicitly imported by the author. |
There was a problem hiding this comment.
What is the motivation/impact of forcing a wire adapter to be imported from another module? Couldn't a component define an adapter in the same file:
function myAdapter() {}
export default class List extends HTMLElement {
@wire(myAdapter) foo;
}There was a problem hiding this comment.
yes! that should be fine but it can't be extracted as metatada. I can update this sentence to reflect that.
docs/proposals/0103-wire-adapters.md
Outdated
| } | ||
| ``` | ||
|
|
||
| * The adapter must be callable that expects exactly one argument. |
There was a problem hiding this comment.
must be callable -> must be a callable
docs/proposals/0103-wire-adapters.md
Outdated
| * The adapter must be callable that expects exactly one argument. | ||
| * The argument provided to the adapter will be the *resolved* value specified as the second argument of the `@wire` decorator. We call this the adapter configuration, and its type could be anything. | ||
| * The adapter configuration is part of the public API specified per adapter. | ||
| * The result of calling the adapter must be a promise. |
There was a problem hiding this comment.
What is the expected behavior wire adapter returns a plain value? Would the wire service takes care of wrapping it into a Promise?
There was a problem hiding this comment.
yeah! but as mentioned above, this proposal is faulty!
docs/proposals/0103-wire-adapters.md
Outdated
| } | ||
| ``` | ||
|
|
||
| * `foo` property will always be an object with a well defined API specified by the `@wire` decorator. |
There was a problem hiding this comment.
What do you mean by:
an object with a well defined API
There was a problem hiding this comment.
oh, I will fix this, it is a well defined schema by the adapter, not by the generic decorator.
docs/proposals/0103-wire-adapters.md
Outdated
|
|
||
| * `foo` property will always be an object with a well defined API specified by the `@wire` decorator. | ||
| * `@wire` decorator is responsible for invoking the adapter (when needed) with the computed configuration. (in this case `'$id'` will be replaced with the value of `this.id` before invoking the adapter). | ||
| * the result of calling the adapter must be a promise, and depending on its resolution, `foo.data` and `foo.error` will be allocated accordingly. |
There was a problem hiding this comment.
What is the expected behavior if the wire adapter throws during invocation? Would the wire service intercept the error and set foo.error?
There was a problem hiding this comment.
that's an implementation detail, but I presume something like this:
new Promise(() => {
return Promise.resolve(adapter.call(...));
}).then((data) => wiredValue.data = data).catch((error) => wiredValue.error = error;)
docs/proposals/0103-wire-adapters.md
Outdated
| * `foo` property will always be an object with a well defined API specified by the `@wire` decorator. | ||
| * `@wire` decorator is responsible for invoking the adapter (when needed) with the computed configuration. (in this case `'$id'` will be replaced with the value of `this.id` before invoking the adapter). | ||
| * the result of calling the adapter must be a promise, and depending on its resolution, `foo.data` and `foo.error` will be allocated accordingly. | ||
| * additional details can be added to `this.foo` by expanding the `@wire` decorator protocol (e.g.: "inflight" flag). |
There was a problem hiding this comment.
I don't understand how the wire adapter would be able to set the inflight property on this.foo.
The wire adapter don't have direct control over this.foo. If the wire decorator who is in charge of setting the value to either data or error.
There was a problem hiding this comment.
the sentence says that it is the wire decorator who should define that, not the adapter :)
docs/proposals/0103-wire-adapters.md
Outdated
| * `foo` property will always be an object with a well defined API specified by the `@wire` decorator. | ||
| * `@wire` decorator is responsible for invoking the adapter (when needed) with the computed configuration. (in this case `'$id'` will be replaced with the value of `this.id` before invoking the adapter). | ||
| * the result of calling the adapter must be a promise, and depending on its resolution, `foo.data` and `foo.error` will be allocated accordingly. | ||
| * additional details can be added to `this.foo` by expanding the `@wire` decorator protocol (e.g.: "inflight" flag). |
There was a problem hiding this comment.
What would be the value of this.foo when the Promise returned by the adapter is in pending state?
There was a problem hiding this comment.
in this proposal, it returns an observable, and I suspect that the value will be undefined until after the first value from the stream becomes available.
docs/proposals/0103-wire-adapters.md
Outdated
| * additional details can be added to `this.foo` by expanding the `@wire` decorator protocol (e.g.: "inflight" flag). | ||
| * `this.foo` object should always be reactive. | ||
| * `this.foo.data` will be read only by the `@wire` decorator protocol. | ||
| * `this.foo.data` will be cached by the `@wire` decorator protocol by using the `config` as its key. |
There was a problem hiding this comment.
I am not a fan of letting the wire decorator caching the value of this.foo.data. It means that we would need to open another API to invalid the cached value. I would rather delegate the caching to the wire adapter implementor.
There was a problem hiding this comment.
Caching shouldn't be an invariant though it is highly recommended for performance reasons.
There was a problem hiding this comment.
either way, I think we need to decide if wire will cache or not since it doesn't know the nature of the adapter, it must take a stand.
docs/proposals/0103-wire-adapters.md
Outdated
| * additional details can be added to `this.foo` by expanding the `@wire` decorator protocol (e.g.: "inflight" flag). | ||
| * `this.foo` object should always be reactive. | ||
| * `this.foo.data` will be read only by the `@wire` decorator protocol. | ||
| * `this.foo.data` will be cached by the `@wire` decorator protocol by using the `config` as its key. |
There was a problem hiding this comment.
Can you expand on how you are planning to cache the config object as a key? Are you planning to hash the config bag?
There was a problem hiding this comment.
the cache is not really about the data, it is about the observable. every time config changes (which we know at the wire decorator level) we will decide if we need to call the adapter again for the new observable, or if we can continue using the existing observable.
There was a problem hiding this comment.
Using multicast observables shouldn't be an invariant but is highly recommended for performance reasons.
But I'm confused about this discussion being under proposal 1 - promises instead of proposal 2 - observables.
There was a problem hiding this comment.
yes, let's focus on proposal 2 and 3, I will put a big note on proposal 1 to avoid confusion
docs/proposals/0103-wire-adapters.md
Outdated
| #### Cons | ||
|
|
||
| * Adapter doesn't really have control, it doesn't know the caller, and it can't do much. | ||
| * The observable protocol is not as trivial as the promise protocol, nor it is standard. |
There was a problem hiding this comment.
Can a callback-based approach solve this issue?
There was a problem hiding this comment.
can you elaborate more?
caridy
force-pushed
the
caridy/rfc-wire-adapter
branch
from
df5e42d
to
edc34ff
Compare
Benchmark comparisonBase commit:
|
pmdartus
changed the title
docs/proposals/0103-wire-adapters.md
Outdated
|
|
||
| ## Summary | ||
|
|
||
| Wire adapters are responsible for providing access to any data that is required by your component, whether it is driven by the external API of the component or by some internal state. New wire adapters are needed to provide new capabilities in terms of data collections. This RFC defines the process to create new wire adapters, their ergonomics and protocols. |
There was a problem hiding this comment.
Internal state that's external to the component, right?
There was a problem hiding this comment.
not sure what are you suggestion?
docs/proposals/0103-wire-adapters.md
Outdated
| ``` | ||
|
|
||
| * The adapter must be a callable that expects exactly one argument. | ||
| * The argument provided to the adapter will be the *resolved* value specified as the second argument of the `@wire` decorator. We call this the adapter configuration, and its type could be anything. |
There was a problem hiding this comment.
We can tighten this to say the configuration's type is object while the shape of that object is anything.
There was a problem hiding this comment.
I would tighten it up as you suggest. I don't think we want people passing functions for configs.
docs/proposals/0103-wire-adapters.md
Outdated
| * the result of calling the adapter must be a promise, and depending on its resolution, `foo.data` and `foo.error` will be allocated accordingly. | ||
| * additional details can be added to `this.foo` by expanding the `@wire` decorator protocol (e.g.: "inflight" flag). | ||
| * `this.foo` object should always be reactive. | ||
| * `this.foo.data` will be read only by the `@wire` decorator protocol. |
docs/proposals/0103-wire-adapters.md
Outdated
| * additional details can be added to `this.foo` by expanding the `@wire` decorator protocol (e.g.: "inflight" flag). | ||
| * `this.foo` object should always be reactive. | ||
| * `this.foo.data` will be read only by the `@wire` decorator protocol. | ||
| * `this.foo.data` will be cached by the `@wire` decorator protocol by using the `config` as its key. |
There was a problem hiding this comment.
Caching shouldn't be an invariant though it is highly recommended for performance reasons.
docs/proposals/0103-wire-adapters.md
Outdated
| * additional details can be added to `this.foo` by expanding the `@wire` decorator protocol (e.g.: "inflight" flag). | ||
| * `this.foo` object should always be reactive. | ||
| * `this.foo.data` will be read only by the `@wire` decorator protocol. | ||
| * `this.foo.data` will be cached by the `@wire` decorator protocol by using the `config` as its key. |
There was a problem hiding this comment.
Using multicast observables shouldn't be an invariant but is highly recommended for performance reasons.
But I'm confused about this discussion being under proposal 1 - promises instead of proposal 2 - observables.
docs/proposals/0103-wire-adapters.md
Outdated
| * A fundamental problem with this proposal is that it does not provide a way for the `@wire` decorator to update the data unless the config changes due to a mutation in the component's instance state or properties. | ||
| * Adapter doesn't really have control, it doesn't know the caller, and it can't do much. | ||
|
|
||
| ### Proposal 2: Observables |
There was a problem hiding this comment.
The behavior I've proposed (see Apex in LWC) and implemented is a combination of proposals 1 and 2:
- Imperative invocation returns a promise.
- @wire invocation returns an observable.
Rationale for the imperative behavior:
- Users are much more familiar with promises than observables. The former is part of the language. Requiring expertise in observables increases the learning curve drastically. As a reference, look at the Angular2 community's struggle with learning RxJS.
- Observables are easy to create leaks and functional issues. Eg subscribe to an observable, next() handler updates cmp state, and then the component is destroyed. The subscription is leaked (only possible to unsubscribe with the return value from subscribe). Subsequent invocation of the next() handler will throw (or be useless) due to the cmp being destroyed. This is a foot gun.
- A promise better models CUD operations. CUD operations will be imperatively invoked because using @wire gives up control flow to the system.
Rationale for @wire behavior:
- @wire is limited to read operations; specifically, read operations that are memoizable (idempotent, side-effect free, etc). This is because using @wire transfers data loading control flow to the system. The system will provision the data to the component it on its timeframe. The invariant is that it happens after component constructor and prop setting.
- @wire avoids leaks and "destroyed component" errors by hooking the lifecycle of the component. This is possible because @wire runs in "privileged space" via the wiring lifecycle hook.
The wire adapter:
a. Must expose an API that returns promises for userland imperative invocation
b. Must expose an API that returns an observable for @wire invocation
c. Ensure consistency of data across those invocation paradigms.
d. Is highly recommended to perform client-side caching. Caching lifecycle is adapter-specific. Utilities to facilitate caching, particularly consistent behavior across adapters, is unspecified.
e. Is highly recommended to use multi-cast observables and other techniques to minimize memory consumption and runtime.
f. Is recommended to provide a cache invalidation mechanism, provided as an imperative JS API that's invocable from userland.
I recommend you review the Apex in LWC proposal at https://salesforce.quip.com/oHydARfHdwaO. We can pull out common requirements into this formalization of wire adapters as needed.
There was a problem hiding this comment.
I have updated the proposal, this proposal from @kevinv11n is proposal 4 now.
adding proposal 2 based on observables adding more details for the proposal 2 based on observables more changes based on the feedback of PR #74
caridy
force-pushed
the
caridy/rfc-wire-adapter
branch
from
edc34ff
to
dd53c4a
Compare
Benchmark comparisonBase commit:
|
docs/proposals/0103-wire-adapters.md
Outdated
|
|
||
| This variation introduces two main problems: | ||
|
|
||
| 1. Combining thenable with observable is itself new and confusing. |
There was a problem hiding this comment.
I really, really would like to have the full power of observables at my disposal. There is a steep learning curve, but I do not think we need to completely remove access to observables in user land. If users are comfortable using Observables, we should let them.
I would challenge that combining thenable with observable is confusing. rxjs has an operator called toPromise that converts your observable to a promise. This behavior has been around for a while and is not confusing.
I think calling .then on our observable could just be an alias for .toPromise. These two would be functionally equivalent:
myObservable.toPromise().then(() => {}).then(() => {})
myObservable.then(() => {}).then(() => {})
One thing that this proposal would need to address is the ambiguity with catch. What happens if a user does myObservable.catch(() => {})? I think this would be the observable flavor because I cannot see why a user expecting a promise would reach for the catch operator here.
There was a problem hiding this comment.
One thing that this proposal would need to address is the ambiguity with catch.
That's why I call it thenable instead of promise. A thenable can be resolved via Promise.resolve if you want to catch it:
var o = { then() { return 1 }}
> undefined
Promise.resolve(o)
> Promise {<pending>}
There was a problem hiding this comment.
I've several issues with exposing observables into userland:
- Userland exposure requires stability and backwards compatibility. The observables spec is stage 1 (source) and undergoing significant churn. Eg yesterday an API-breaking change was proposed.
- The observables spec hasn't defined operators. The spec authors are focused on the interface now, operators will come in the future (source). Once the interface is further along (see 1 above) then operator libraries will support it, and then userland can bring its own operator library (we won't ship one due to Salesforce's support requirements).
- The foot gun of leaking subscriptions is unsolved. This needs exploration.
Thennable observables have been brought up elsewhere but have some challenges (eg Promise<thennable Observable> is fun). We're mostly concerned with Observable->Promise so this may not be an issue. Regardless the debate is on-going for whether the observable, subscription, or something else is thennable (and thus await-able).
toPromise was punted until the spec is finalized. That's really what you're asking for.
Observable.forEach gives a Promise<void> when the observable completes/errors. It's implemented in RxJS5. But this solves a different problem: wait for the observable to finish vs capture the first emitted value.
If you read any of the above links you'll see how much debate is still on-going with observables. That reinforces my concerns with exposing them to userland in the immediate future.
The benefit of proposal 4 is the reduction of the audience wire adapter implementers <<< userland devs. It doesn't eliminate issues 1 and 2 above but greatly reduces the impact and audience we must work with to move them forward as things change. Proposal 4 is forwards-compatible with proposal 3.
I recommend we proceed with proposal 4 now and monitor observable's standardization to unblock proposal 3 in the future.
docs/proposals/0103-wire-adapters.md
Outdated
|
|
||
| #### Open Questions | ||
|
|
||
| * should the config object be a simple `Record<string, string | number>`, or could it be a complex/multilevel object? |
There was a problem hiding this comment.
What is Record here? Do you mean Object?
There was a problem hiding this comment.
Oh, Record is a typescript concept for hash tables.
docs/proposals/0103-wire-adapters.md
Outdated
|
|
||
| This variation introduces two main problems: | ||
|
|
||
| 1. Combining thenable with observable is itself new and confusing. |
There was a problem hiding this comment.
I've several issues with exposing observables into userland:
- Userland exposure requires stability and backwards compatibility. The observables spec is stage 1 (source) and undergoing significant churn. Eg yesterday an API-breaking change was proposed.
- The observables spec hasn't defined operators. The spec authors are focused on the interface now, operators will come in the future (source). Once the interface is further along (see 1 above) then operator libraries will support it, and then userland can bring its own operator library (we won't ship one due to Salesforce's support requirements).
- The foot gun of leaking subscriptions is unsolved. This needs exploration.
Thennable observables have been brought up elsewhere but have some challenges (eg Promise<thennable Observable> is fun). We're mostly concerned with Observable->Promise so this may not be an issue. Regardless the debate is on-going for whether the observable, subscription, or something else is thennable (and thus await-able).
toPromise was punted until the spec is finalized. That's really what you're asking for.
Observable.forEach gives a Promise<void> when the observable completes/errors. It's implemented in RxJS5. But this solves a different problem: wait for the observable to finish vs capture the first emitted value.
If you read any of the above links you'll see how much debate is still on-going with observables. That reinforces my concerns with exposing them to userland in the immediate future.
The benefit of proposal 4 is the reduction of the audience wire adapter implementers <<< userland devs. It doesn't eliminate issues 1 and 2 above but greatly reduces the impact and audience we must work with to move them forward as things change. Proposal 4 is forwards-compatible with proposal 3.
I recommend we proceed with proposal 4 now and monitor observable's standardization to unblock proposal 3 in the future.
docs/proposals/0103-wire-adapters.md
Outdated
|
|
||
| #### Addressing issues from proposal 3: | ||
|
|
||
| * The API that returns an observable doesn't have to be exposed to user-land, instead can be just registered, maybe via `wire.registerAdapter(publicPromiseBaseAPI, privateObservableBaseAPI)` in the module that defines both functions, which guarantees that users will access the `publicPromiseBaseAPI`, and that's what they will provide as the identify of the adapter via the `@wire` decorator, while the internals of the `@wire` decorator can invoke `privateObservableBaseAPI` instead to obtain access to the observable. |
There was a problem hiding this comment.
I'd like to explore eliminating the need for wire.registerAdapter. Perhaps an importable symbol can be used so the wire service can access the privateObservableBaseAPI while userland can't. We can use import static analysis to control access of the importable symbol.
There was a problem hiding this comment.
I'm fine with 4 if the registration API is private for now (only for whitelisted folks with the condition that we will break them at some point).
I will not count on the observable standardization, at least not as part of the language. Again, the observable protocol is not part of this proposal, that will require its own proposal all together.
There was a problem hiding this comment.
We haven’t discussed the protocol. We’re discussing the interface that defines an observable and that is in flux. That’s an issue because it’s intrinsic to the API we’re defining.
I’m less concerned with observables being part of th language than with it being a stable spec adopted by a couple major implementers. To that end, AFAIK TC39 has asked the observable spec group to work with WHATWG and that’s in progress (see whatwg/dom#544). Let’s give that some time to progress.
There was a problem hiding this comment.
See a 5th proposal I've added in a comment above. I believe it allows for the elimination of wire.registerAdapter, remains agnostic to other specs or standards, avoids reliance on the Observable spec, allows wire developers to use standard or non-standard Observables from any library they please, and allows for Promises or any other mechanism.
docs/proposals/0103-wire-adapters.md
Outdated
|
|
||
| The primary use-case for the `@wire()` decorator is to allow access to any data, not just salesforce data, in a data-stream fashion, while using the public or private state of the component as the inputs for the wire adapter specified via this decorator. | ||
|
|
||
| The challenge is to be able to define new adapters to expose new capabilities without too much hazard while preserving all existing the invariants. |
There was a problem hiding this comment.
"preserving all existing the invariants." → "preserving all existing invariants."
docs/proposals/0103-wire-adapters.md
Outdated
|
|
||
| Wire adapters are responsible for providing access to any data that is required by your component, whether it is driven by the external API of the component or by some internal state. New wire adapters are needed to provide new capabilities in terms of data collections. This RFC defines the process to create new wire adapters, their ergonomics and protocols. | ||
|
|
||
| ## Motivation |
There was a problem hiding this comment.
It might be worth mentioning that data can also come from imperative access, and that a big motivation for using @wire over an imperative method is that it will handle automatic rewiring and rerendering of the UI. If everything were only available imperatively then this would not be automatic, and inconsistent reactivity would result.
docs/proposals/0103-wire-adapters.md
Outdated
| #### Cons | ||
|
|
||
| 1. Adapter doesn't really have control, it doesn't know the caller, and it can't do much. | ||
| 2. The observable protocol is not as trivial as the promise protocol, nor it is standard. |
There was a problem hiding this comment.
I see 2 issues with the current proposals.
- The latest decorator proposal allows access to the instance within the initializer. Some adapters may need to communicate using events as part of their implementation (e.g. contextual values).
I think the proposal needs to be augmented to provide the component instance or capabilities related to it to the wire adapter itself. If there's too much concern to expose the instance directly, the framework could expose an EventTarget spec proxy to the wire adapter which encapsulates event-related capabilities of the instance.
But are there other capabilities of the component that would be important to provide as well? What about connected/disconnected? That leads to a second issue.
- What dependencies should we build into and rely on intrinsically within the wire spec? Will the component's capabilities be hidden from the adapter and only used implicitly through some other means such as through a returned Observable's life cycle? The more that's hidden and abstracted, the more the system creates a dependency upon other semantics, like the Observable's.
There is wisdom in designing the wire adapter's protocol to have fewer dependencies on other specs. Can we consider a proposal that allows someone to build Observable-semantics on top of a wire protocol that is independent of the stage 1 level Observable?
For instance, I'd like to propose option 5 as follows:
wireAdapter(instance, propertyGetter, propertySetter) {
return {
wireChangeCallback: function(config) {
// Called after resolving the @wire arguments or whenever it changes
},
connectedCallback: function() {
// Called when the instance connectedCallback is invoked
},
disconnectedCallback: function() {
// Called when the instance disconnectedCallback is invoked
}
};
}That allows the wire to do the following:
- Use the instance if necessary (fire events, etc)
- Read/write the wired property's value whenever it wants
- Be notified whenever the resolved value of the
@wirearguments change (e.g. a referenced property value changes) - this part is proprietary to@wire - Be notified whenever the component is connected/disconnected - this part uses the same standard as the Web Component itself
This proposal imposes no additional dependencies on things which aren't standard beyond the proprietary @wire itself, but leaves the protocol as close to the metal as possible.
There was a problem hiding this comment.
I do entertain the idea of having an indirection here that allows people to switch between different observables, specially because that will allow people to use other popular systems like Redux. It is important to notice that this flips the control, now it is the adapter who should be in control (probably better anyway). So, overall I do agree! But, there are two main issues here:
- lwc proprietary API footprint will increase! not super concern here because it is not really the engine, but the decorator.
- adding the component, or the element or any DOM specific thing to this API makes it less attractive. one of my goals was to create something that could be used with lwc, but also with any other UI system (which is kinda a validation mechanism for our design). But adding an element to the mix might defeat all that.
I think it is time for another meeting about this topic!
and add support for refresh capabilities.
Benchmark comparisonBase commit:
|
docs/proposals/0103-wire-adapters.md
Outdated
| - `targetSetter` is a callable used to emit new wired values. | ||
| - `updatedCallback` is invoked when the resolved configuration changes. The resolved configuration is the sole argument. | ||
| - `connectedCallback` is invoked when the component is connected. | ||
| - `disconnnectedCallback` is invoked when the component is disconnected. |
|
|
||
| The wire service remains responsible for resolving the configuration object. `updatedCallback` is invoked when the resolved configuration changes. The configuration has type object. Its keys and values are type any, and are specific to the wire adapter. This object must be treated as immutable. | ||
|
|
||
| The wire adapter is responsible for emitting the wired value with `targetSetter`. `targetSetter` handles property assignment or method invocation based on the target of the `@wire`. The wired value semantics and shape are unchanged: `{ data: any, error: any }` and only one of `data` and `error` may be non-null. |
There was a problem hiding this comment.
Allowing a wire adapter to emit this form makes sense, but I don't think the wire service should impose/enforce/check it. What the adapter emits is opaque to the service. Many/most of our standard/internal wire adapters will do this (e.g. LDS), but some probably should not (e.g. flexipage region width) given the increased complexity and lack of usefulness of error in those cases.
|
|
||
| Imperative access to data is unchanged. The wire adapter module must export a callable. The callable's arguments should (not must) match those of the wire adapter's configuration. The return value is adapter specific; it need not be a promise. | ||
|
|
||
| ### Refresh |
There was a problem hiding this comment.
This section seems to be more exemplary in nature than specification-oriented. In other words, it provides an example and pattern for implementing other things a wire adapter module may export that provide other capabilities in conjunction with the wired value, refresh being the canonical case used here.
docs/proposals/0103-wire-adapters.md
Outdated
| ### Advantages | ||
|
|
||
| - The wire adapter has control of the types of data and error it emits. | ||
| - The wire adapter has control over how it emits read-only values, enabling optimizes specific to the adapter. |
| - Wire adapters require registration to support the declarative `@wire` syntax. | ||
| - Adapter registration can happen after application boot by importing `register` from `wire`. | ||
| - Non-registered wire adapters could still function with `@wire(getType)` if the wire service uses `getType` for a one-time resolution. | ||
| - Refreshing a wired method requires capturing the wired value. This burden is considered acceptable because it's little code and wiring methods is an advanced use case. |
There was a problem hiding this comment.
This is insightful. For modules with wire adapters that also export a refresh method, we should probably provide sample code in documentation to show how that refresh should work when the @wire is bound to a method.
An alternative API could specify that the first parameter to the refresh method is the actual reference to the @wire member directly, whether it's a field or a method.
import { getType, refreshType } from 'ns-type';
@wire(getType, A_Args)
aType;
@track
capturedBType;
@wire(getType, B_Args)
onBType(value) {
this.capturedValue = value;
}
refreshAll() {
refreshType(this.aType); // No confusion to the developer here
refreshType(this.capturedBType); // Option 1: The API targets the emitted value passed to the method
refreshType(this.onBType); // Option 2: The API targets the wired thing (the method), not the emitted value
}I'm not sure which is more intuitive. I do know that this alternative of passing the wired thing directly instead of the emitted value does have implications on the register API since targetSetter is actually the normalized function and the current spec abstracts it, which is very nice.
There was a problem hiding this comment.
Option 2 incurs a performance overhead because the method is defined on the prototype, not the instance, so we'd have to clone the method to each instance.
Benchmark comparisonBase commit:
|
|
I have a thought regarding this topic, instead of asking wire adapters to import {getTodo} from 'todo-wire-adapter';
export default class Foo extends Element {
connectedCallback() {
getTodo[ConnectedSymbol]();
}
disconnectedCallback() {
getTodo[DisconnectedSymbol]();
}
}Then in your adapter, you just need to import those symbols from import { wireSymbol, connectedSymbol, disconnectedSymbol } from 'wire'
export function getTodo(config) { ... }
getTodo[wireSymbol] = (newConfig) => {...};
getTodo[connectedSymbol] = () => { ... };
getTodo[disconnectedSymbol] = () => { ... };The benefit of this approach is that an adapter can just implement things that are used. If no one is using the lifecycle hooks, the adapter author doesn't need to implement it, and no factory or utility function is needed. The disadvantage is that the lifecycle hooks are exposed and could be invoked if one intends to. |
|
|
||
| An implementation of the `todo` wire adapter that uses observables for a stream of values. | ||
|
|
||
| ```js |
There was a problem hiding this comment.
In this example can you expand on the role of releaseConfig and captureWiredValueToConfig
docs/proposals/0103-wire-adapters.md
Outdated
| import { register, targetGetter } from 'wire'; | ||
|
|
||
| // Difference: receive a dispatchEvent, use DOM Events to fetch the observable | ||
| getObservable(dispatchEvent, config) { |
There was a problem hiding this comment.
getObservable(dispatchEvent, config) { -> function getObservable(dispatchEvent, config) {
docs/proposals/0103-wire-adapters.md
Outdated
| } | ||
|
|
||
| // Difference: receive eventTarget | ||
| register(getTodo, function wireAdapter(targetSetter, eventTarget) { |
There was a problem hiding this comment.
Is the eventTarget the component instance, the host element or an opaque object?
| - `register ` is provided by the wire service module. It receives two arguments: the wire adapter id and _wire adapter_. Return type is null. | ||
| - `getType` is the wire adapter id (ie imperative accessor for the data type, defined by the wire adapter). Components use this like `@wire(getType, {...})`. | ||
| - `wireAdapter` is the _wire adapter_. It is invoked per @wire instance (which is per component instance). It must return an object with zero or more of the defined callbacks (see below). | ||
| - `targetSetter` is a callable used to emit new wired values. |
There was a problem hiding this comment.
Here is my 2 cents comment.
Since targetSetter is a callable, I think it would be interesting to have a callback example to introduce the concept of wire adapter. Observable is complicated construct and I am suspecting that most of the developers are not familiar enough (including me I admit) to weight all the nuances.
Benchmark comparisonBase commit:
|
docs/proposals/0103-wire-adapters.md
Outdated
| .toPromise(); | ||
| } | ||
| // Wire adapter id isn't a callable because it doesn't support imperative invocation | ||
| export Symbol('getTodo'); | ||
|
|
||
| // Difference: receive eventTarget | ||
| register(getTodo, function wireAdapter(targetSetter, eventTarget) { |
There was a problem hiding this comment.
don't you need to use Symbol('getTodo') as the identifier here?
Details
First draft of the proposal for the wire adapter formalization and protocol.