-
Notifications
You must be signed in to change notification settings - Fork 12
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
[context] are context objects needed? #19
Comments
This was actually how the initial proposal was made. The issue with this is it's incredibly 'loose', and it puts the responsibility of checking types on the consumer. With a well known and published context object we have an actual dependency that can be documented and clearly typed and shared. This tightens up the protocol a little bit and I think is generally worth the extra overhead. |
Thanks for the response @benjamind. Can you explain more? I'm not sure I understand, as DOM events have types, for example a click is a MouseEvent. I'm no TypeScript expert but it seems possible to map an event name to the typed event class, such as is done here: https://github.com/microsoft/TypeScript/blob/main/lib/lib.dom.d.ts#L5562. I don't see why these event classes can't be shared the same way that a context object can. Some downsides to the current context object approach is that it repeats some things that are built in to DOM events:
|
The tradeoff here is that with separate event names you have to add event listeners for each context type, since you can't add wildcard event listeners. That in turn makes it hard to abstract over the context protocol. You can't make a React bridge as easily, or forward all context requests to a different subtree (as you might want to do with portal/projection patterns). It also means that the context key is a string - the event name suffix, and this is what Ben was alluding to. With a string key I think we run the real risk of collisions. How many incompatible |
Thanks @justinfagnani for clarifying those points. @matthewp to address some of your other questions. Yes you could use an interface map to type a string key, but that then would require a dependency on a common package to ensure those types are properly followed (the map has to be defined somewhere) so this doesn't gain us any convenience over the object approach. Your other concern regarding The event apis don't really give us any mechanism to do this unless we modified the protocol to use events going in both directions (i.e. a consumer dispatches an event to request a context, a provider is returned in the events callback/payload, the consumer then attaches a listener to the provider, and the provider then emits events on change). This is indeed another approach the protocol could take, but I fear it produces an explosion in events, and more complexity in hookup for the basic case. |
I'd love to see the proposal updated with specifics of these use-cases. The current approach adds a lot of boilerplate over event names and I think that complexity needs to be justified with explicit use-cases.
The current proposal has the same change of collisions. The context |
With the current proposal there is no chance of collisions as Providers check the provided context object for reference equality. They do not introspect into the name property of the object, that is only provided as a debugging convenience. |
Yeah I agree, they seem equally able to be typed, in which case I think the more simple and webby approach, events, is the way to go here.
Yeah, I think this is perhaps better discussed in a separate issue. The callback approach makes it seem like the event is handled asynchronously. Another approach is for the provider to add a property (or properties) to the event. That could contain the context value and an eventtarget to listen to for changes to the value. I'll file another issue to discuss this idea. |
Ok, I got it, they need to have a reference to the same object in memory. This makes the API more complex in my mind. We have these things that seem to conflict in my mind, or be too close to serving the same purposes:
There are a lot of moving parts here. I think a much simpler version of this proposal can exist by removing the context object which is a source of most of the complexity. Just events and properties added to the event. I don't think name collisions justifies the added complexity and duplication of logic that events already provide ( |
The
With this approach we have much more complicated logic for handling updating contexts, or contexts which are provided asynchronously, we essentially have two paths through which context is provided. We also have many more events being created and dispatched when contexts are updated frequently (Events are not free). Consumers also need to still manage a reference to the EventTarget so they can later unsubscribe, so this is equavalent to the This change does not solve the collision problem either, and since you still need to require a dependency to get strong typing on the string keys, it has not reduced dependencies or complexity in usage. While I agree we do already have to manage namespace collision issues in the Web Components world, I do not think we should add to this burden if it can be avoided! I do not feel this proposal simplifies anything over what is currently in the proposal, and I think there's additional runtime overhead to the event subscription model being proposed that could likely become an issue in some high frequency update scenarios. I agree that the |
Complexity is a problem if it's significant and doesn't pay for itself, but in this case I really do think it does. String-based name collisions aren't just possible, but likely. The object-key API is not unique to this context protocol, and it's really not that much more "complicated". This is very similar to the React API (https://reactjs.org/docs/hooks-reference.html#usecontext), and complexity isn't a complaint I hear about that. In React you create a context:
export const loggerContext = React.createContext(defaultValue); and can then retrieve a context value with it: import {loggerContext} from 'x-react-logger';
const MyComponent = () => {
const logger = useContext(loggerContext);
// ...
}; That just doesn't seem complex to me, and you'll be able to do the same patterns with the context protocol:
export const loggerContext = Symbol('logger'); import {loggerContext} from 'x-wc-logger';
class MyElement extends LitElement {
_logger = new ContextConsumer(this, loggerContext);
} |
Things appear less complex when you omit code required to implement them 😀. I assume the undefined this.dispatchEvent(
new ContextEvent(coolThingContext, (coolThing, dispose) => {
// if we were given a disposer, this provider is likely to send us updates
if (dispose) {
// so dispose immediately if we only want it once
dispose();
}
// guard against multiple assignment in case of bad actor providers
if (!this.myCoolThing) {
this.myCoolThing = coolThing; // do something with value
}
})
); That is very complex. Think of scenarios where you need the My proposal would be: let event = new ContextEvent('cool-thing');
this.dispatchEvent(event);
// The current value.
console.log(event.context.value);
// The context is an EventTarget
event.context.addEventListener('change', () => {
// do something with the new value, if you want.
console.log(event.context.value);
});
// Get the next value but only once. This is free because its an EventTarget, no special API required.
event.context.addEventListener('change', () => {
// Get this new value.
}, { once: true })
I don't understand this argument in the context of HTML/DOM based APIs where string keys are used everywhere. Should custom elements not use events at all then? After all they could be named the same as other events. If I emit a Instead we use the usual method of preventing conflicts when we fear them; namespacing. That applies here the same as anywhere else (other events, custom element names, etc). The event name |
The proper solution for event name collisions is upstream changes to DOM to allow symbols as event name. There's an existing issue where a maintainer expressed interest: whatwg/dom#331. You could do something like: provider.js export const eventName = Symbol('some-thing');
this.addEventListener(eventName, () => ... import { eventName } from 'https://example.com/some/shared/place/events/some-thing.js';
this.dispatchEvent(new ContextEvent(eventName)); |
We're getting a little off topic here (we're supposed to be discussing context objects as key vs strings, we're now on the topic of callback vs event driven approaches). Having said that, lets try and get a little more specific about the proposed change... Your proposal is using EventTarget provided by the Provider inside the event, with context value being set into the event: import { coolThingContext } from 'cool-thing';
class CoolThingElement extends LitElement {
constructor() {}
connectedCallback() {
super.connectedCallback();
// we want a cool thing
let event = new ContextEvent(coolThingContext);
this.dispatchEvent(event);
// we have to check the context was set, since it might not have been handled
if (event.context) {
// it was set on the event context property
this.coolThing = event.context.value;
// we need to store the context event target so we can unsubscribe
this.coolThingContext = event.context;
// and we need to store the function so we can correctly unsubscribe
this.onCoolThingChanged = (value) => { this.coolThing = value; }
this.coolThingContext.addEventListener('change', this.onCoolThingChanged)
}
}
disconnectedCallback() {
super.disconnectedCallback()
if (this.coolThingContext) {
this.coolThingContext.removeEventListener('change', this.onCoolThingChanged);
}
}
} And our current proposal: import { coolThingContext } from 'cool-thing';
class CoolThingElement extends LitElement {
constructor() {}
connectedCallback() {
super.connectedCallback();
// we want a cool thing
let event = new ContextEvent(coolThingContext, (value, dispose) => {
// we were given a value in the callback
this.coolThing = value;
// we have to store dispose so we can use it later
this.coolThingDispose = dispose;
});
this.dispatchEvent(event);
}
disconnectedCallback() {
super.disconnectedCallback()
if (this.coolThingDispose) { this.coolThingDispose(); };
}
} Its incorrect to think that EventTarget results in less complexity at the point of use. You still need to handle the unsubscribe, and you need to hang onto the contexts provided EventTarget to do that as well as the subscribed function. While It also has a significant performance advantage. Here's a rudimentary benchmark comparing the use of EventTarget vs callbacks.. On my run of this callbacks come out around 30% faster than using events, and 15% faster than events which reuse the event object (a potentially confusing pattern?). But this doesn't tell the whole story, taking a look atthe profile below we can see that the event driven approaches result in spiking memory usage and garbage collection, while the callback approach has stable memory usage. We are already somewhat concerned with the usage of events to setup context in large hierarchies of context dependent components. Using events further to drive updates is only going to increase the problem. Overall while I heartily agree that protocols should try and hew as close to familiar browser patterns as we can, in this case I feel the proposal as currently stated is the most simple and efficient we can currently get. |
If we want to optimize for the 'get a context once' case, then we might consider combining some of these ideas: class CoolThingElement extends LitElement {
constructor() {}
connectedCallback() {
super.connectedCallback();
// we want a cool thing
let event = new ContextEvent(coolThingContext);
this.dispatchEvent(event);
// if we only want the value once, then retrieve it from the event object
this.coolThing = event.value;
// but if we want updates, then we can pass the callback:
this.dispatchEvent(new ContextEvent(coolThingContext, (value, dispose) => {
this.coolThing = value;
this.coolThingDispose = dispose;
})
}
disconnectedCallback() {
super.disconnectedCallback()
if (this.coolThingDispose) { this.coolThingDispose(); };
}
} This complicates the Provider somewhat since it needs to do both checking for a given callback in the event, and setting the value into the event, but this does make the 'once' case simpler on the part of the Consumer, and we can then make I personally think the main use for Context is a 'live binding' with multiple update, and suspect the 'once' case is actually rare. If we look at prior art on this such as React Context they only support the live binding from what I can tell, all Consumers are updated when the Providers value changes. Thoughts on this @justinfagnani ? |
The goal of the protocols is to allow interoperable implementations. It is not necessarily to be used without any implementations at all. Just as React, Angular, etc., provide sugary APIs for interacting with their underlying Context/DI implementations, we presume that libraries - whether associated with a web component helper library like Lit, Stencil, etc., or not - will provide nicer APIs than raw events. It's also a goal of mine, though lower priority, that at least that the raw protocol not be too difficult to use by hand, so striving for as much simplicity there as possible is good, but critical functionality and interop are higher priority than that. Regarding string names vs object reference though, this is simply a matter of safety and correctness. Strings have value semantics and you can't be sure that two independent modules won't accidentally use the same string. References fix this with the only added complexity coming from needing to get a hold of the reference. This will usually be an import, which has the benefit of providing some kind of reference back to an object that can be documented as to what the context's type and contract are. I suspect with string keys we'd see that pattern often anyway, making the complexity complaint moot. |
The problem I see with depending on object references is that we can't pass variables to it. Let's say my How can we tell to the provider that the requested I first thought about augmenting May be we need context objects + payload objects sent in the |
I feel like in this case you want to request via context a factory function, which can then take the arguments and return the appropriate logger. i.e. if you want an object to be returned parameterized, the provider is a provider of functionality which enables this, not a mechanism by which the parameters themselves are passed. The object reference is only for identifying the provider and doesn't restrict what could be provided. |
Oh damn that's great! I've not thought about it that way, a factory function is indeed the best way to do that. Thank you for the clarification 🙏 |
(new here, experimented a for a while before joining here. Gotta read all of what's around on the topic) I get we should use See: Snippet of an experiment using CustomEvent and use them with this proposal. export interface AnotherEventContext {
// ...
}
declare global {
interface HTMLElementEventMap {
readonly 'context-request:another-event': CustomEvent<AnotherEventContext>
}
} Making use of a plain But for the subscribtion. The providers, how is it planned we set them up? Should we have the Provider catch the context, and from that side (e.g. in the project code, no longer from lit adapter, or etc.) and from there, add other project side related DOM events, and how to cleanup etc? Something like "respond for" and "keep track" and "unregister". Like a "Stateful Context Manager" (see Gist) That's what's still vague to me. — Probably I should read up more here :) |
@renoirb the proposal already uses a subclass of Event: https://github.com/webcomponents/community-protocols/blob/main/proposals/context.md#the-context-request-event This is a separate topic from the context object question though, so I'd suggest a new issue if that doesn't answer your question. |
Yes, I understood that bit. It was my way of giving context (pun not intended) to my question regarding what we pass in that Event. We basically pass more than a plain-old-javascript-object. And we use what we pass in so that provider has access to mutating that consumer's (component) internal state. Consumer components might expose private methods, etc.
My question is still related about the Provider. Its lifecycle, how we should work when we're implementing one. I see @benjamind comment made on July 7th makes sense. (moved to #21 in #21 (comment)) I won't continue in this thread if you're still certain of what I'm saying doesn't make sense. Maybe that should be a different thread. If so, please guide me in creating a re-worded description of this, please :) |
I think that using object references could be problematic in some scenarios. In my case we are building a big app composed by a shell app and several verticals as microfrontends (using web components), having each vertical its own build. Ideally, verticals could have context consumers while the shell app could contain the context providers. With object references all verticals must be aligned in order to use the same context references. Otherwise, consumers are going to ask for a non existent provider because the context reference is different between the vertical and the shell app. I know this could be a strange use case now butr, the use of tools like skypack could make this use case more common in a near future. |
That's a good callout. If the context object key was made using |
Indeed, that would work 👌 |
Great, I can definitely make that change to the proposal. Will try and file a PR to that effect soon. |
@benjamind did you have time to try this? I've been playing around using So I think we could use something external to the protocol like a global registry for those cases. export const getMycontext = () => getGlobalRegistry().get('my-context'); |
Right yes had some time for hunk on this some more and my initial suggestion for Symbol.for will cause conflicts to become an issue once again. In discussion with @justinfagnani we concluded that the context request protocol should probably be non specific in what a context key should be. Some use cases may prefer global uniqueness with the downsides of potential conflict, others may prefer value semantics with the need to resolve shared keys via common imports. It feels like there is not a one size fits all solution here? |
Makes sense. Thanks! |
Note that things are progressing smoothly on the symbol-as-event-name front: whatwg/dom#331 (comment) |
A bit late here but chiming in from FAST's perspective. We're working on integrating Context with our Dependency Injection system. In order to make that work, we prefer a single event that we can register the container for, and a specific context object that we can use to look up in the container. With an unknown list of events, things would be near impossible to handle in a generalized way. We'd have to force a ton of setup on consumers and would likely defeat the point of some of this. A bit more technical details on our implementation: the DI system uses a export type Key = PropertyKey | object | InterfaceSymbol | Constructable | Resolver; The |
The title of this issue, though not the multiple event names discussion, highlights an important discrepancy between the protocol as checked in and the In export type ContextKey<KeyType, ValueType> = KeyType & {__context__: ValueType}; This actually seems sufficient for al use cases I can think of (except that it needs a brand property typed as Should we remove the Context object from the proposal and say a context can be any value? Or should we update An important question for me is whether there are concrete and critical use-cases for initial value that can't be met by the provider. cc @benjamind |
My feeling is that a default value as defined by a context key isn't really that useful overall. Defaults I have found to be component-local for the most part (what the component would use if the context was never provided). This is especially true where contexts are treated more like interfaces to be implemented by the providers, in which case what would a default value really be as defined by the interface? You can also easily define a default value by simply having a default context provider at the root of your tree that would be superceded by a later registered context provider. So my vote is to update the protocol to this, but I have to admit my head has not been in this space for a long time now. |
From my perspective a context key should be treated as an opaque token/value by the spec and probably context API implementations as well. |
This is already the case in the Lit implementation, and I have a PR up to codify that change: #36 |
Oh, @manolakis, if only I could have said exactly those words back when I came here. This was exactly what I was trying to explain and was working with (I no longer work there anymore though). How verticals can declare their shapes, and the shell to fulfill them. |
Reading the Context proposal one thing that sticks out to me is that it's a bit unclear what the purpose of the Context object is. It seems like it's used by a provider as a filter for contexts that it can supply. Assuming that's the case, wouldn't make the type of context being requested part of the event name instead?
ie, instead of
"context-request"
it becomes"context-request-theme"
and a theme provider listens to that specific event rather than all context events.The text was updated successfully, but these errors were encountered: