-
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] do providers retain references to consuming components? #21
Comments
I think you have the right idea. It may be helpful to check out the lit-context implementation I've started here. Especially this ContextProvider implementation. Assuming you wish to create a Context Provider which can deliver the context more than once, i.e. when its context value changes, it will have to hold onto the callback function which is provided in the ContextEvent for any context it is interested in satisfying, which also has the It shouldn't retain the Event object, only the callback from within it. Whenever the context value in the provider changes (not properties on the context value, the value reference itself), the provider should call the callbacks it has collected again. One thing that I think is wrong in your implementation here is that your provider is driving the Additionally, if a ContextProvider intends to satisfy context requests with new values like this, it should pass a https://github.com/webcomponents/community-protocols/blob/main/proposals/context.md#usage |
Moving here what was in #19 (comment) for #19
(...) /**
* In experiment above, this was called UpdatableHonk — Naming things are hard.
* Pretty much an event with a target property.
*/
export interface IContextEventWithTarget<T extends UnknownContext> extends IContextEvent<T> {
readonly target: EventTarget
}
export class StatefulContextManager {
contexts = new Map<string, Set<IContextEventWithTarget<Context<unknown>>>>()
// Partially inspired by Microsoft Fast Foundation listenerMap
// packages/web-components/fast-foundation/src/utilities/match-media-stylesheet-behavior.ts
private listenerMap = new WeakMap<EventTarget, ContextCallback<unknown>>()
respondFor(name: string, data: unknown) {
const contexts = this.contexts
if (contexts) {
if (contexts.has(name) === false) {
throw new Error(`StatefulContextManager respondFor: There is no context on the name ${name}`)
}
const entries = contexts.get(name)
for (const { context, target, ...rest } of entries) {
const callback = this.listenerMap.has(target) ? this.listenerMap.get(target) : void 0
const payload = data ?? context.initialValue
callback(payload)
}
}
}
protected keepTrackContextRequest<T extends UnknownContext>(event: ContextEvent<T>) {
const { context } = event
const { name } = context
const contexts = this.contexts
const callback: ContextCallback<ContextType<T>> = (value, dispose) => {
event.callback(value, dispose)
// communicate somehow that we've been into a change callback. Maybe?
}
// ... some way of calling the call back and updating
const honk: IContextEventWithTarget<T> = {
multiple: false,
...event,
callback,
context,
target: event.target,
}
this.listenerMap.set(event.target, callback)
if (contexts) {
if (contexts.has(name)) {
contexts.get(name).add(honk)
} else {
contexts.set(name, new Set([honk]))
}
}
event.stopPropagation()
}
/**
* So we can make the Provider to add the handler (remove below)
* Making this Stateful Manager separate from the providers themselves.
*/
addEventListenerTo(host: HTMLElement) {
host.addEventListener('context-request', this.keepTrackContextRequest.bind(this))
}
removeEventListenerTo(host: HTMLElement) {
host.removeEventListener('context-request', this.keepTrackContextRequest.bind(this))
}
} (Added while making this comment on this thread) Not necessarily to do updates, like we see below with
Right.
Right. And that callback may have access to internal state.
Exactly, we might still need some orchestration between consumer/provier, for making sure we properly Because, in the end, a provider would still have to know which components are "context-request" friendly, and may want to ask some registry that something needs to be changed. How would that be done? |
No a provider doesn't need to lookup which components are context-request friendly. This is actually why moving to the context object as a key for the context event request is valuable. When a component needs a context value, it will fire the ContextEvent, in that event payload is an object that is the shared agreed upon identifier for context values of the type the requesting component wants. Therefore if a provider which can satisfy that request intercepts the ContextEvent, it can be reasonably confident that the component that issued the event is playing by the Context API rules and will do the right thing with the value provided to the callback function that was attached to the event. So there's no need for a registry, but there is need for a shared agreed upon object that is used to key the context-request event. |
(After having scavenged in notes) What I've been talking here feels similar to what lit/lit#1955 is doing. You have an abstract "Provider" and "Consumer" (which also cleans after itself). Also a "ContextContainer" to handle telling that things has to be updated. — Which is close to what I've called here StatefulContextManager, hehehe, like benjamind/lit-context 🖖🏾 or DeadWisdom’s Gist, there's also @kcmr ’s kcmr/context-provider that looks similar too. I hope my messy comments makes sense. In the links here, we might want an initial implementation that can be used as a starting point. |
OK! Makes sense. That's why you have in benjamind/lit-context
Right, we're doing the binding ourselves in both places instead of something doing it. |
If we consider making a dependency less, not bound to a framework package that provides Context API, we might want to provide a ContextProvider or ContextContainer. At least the boilerplate of it. When making a package, we would ask package users to use the baked container,provider,consume in a way that would need to be extendable. We could either provide that boilerplate as a class mixin or as "state manager" the component would instantiate at connectedCallback, or constructor time. Probably we could do the same for context-provider or context-container. So we have two bits who knows how to tie themselves together. That side might be as useful as a "shared agreed upon object that is used to key the context-request event" because it would be the perfect place to enforce the contract. What would that package do for this purpose? PS: I tried to look for ContextAPI and web component communication protocol in W3C WICG discussion forum discourse.wicg.io , and haven't found anything and could be a good place to have a conversation. So that issue tracker conversation would be specification proposal draft process not a free-form conversation like I've been doing. |
Shoot, I missed today's meeting. I'll make sure it's in my calendar so I don't miss BREAKOUT: Community Protocols next time |
After more experimentation. Keeping the callback reference for reuse makes sense. What can we do if we have a good amount of Context callback in memory for the same Context. But each owner of that context reference might have a discriminant of some sort. Say it's a property with an ID. The Ideas? Adding a concrete example I wished I've brought up earlier, I've added it as comment in lit-labs context PR What if we want many // Some list of IDs we already have
const ids = ['foo', 'bar', 'bazz']
// Iterating
const template = html`<simple-context-provider>
${repeat(
ids,
(id) =>
html`<simple-context-consumer
data-discriminant-id="${id}"
></simple-context-consumer>`,
)}
</simple-context-provider>` That pattern would be useful in a micro-front-end app that knows a list of ids, but can't know data to fill each |
I feel like this is answered in the description of Context Providers here: https://github.com/webcomponents-cg/community-protocols/blob/main/proposals/context.md#context-providers To quote an answer I gave in #39 :
|
In #40 I clarify the language to say that a provider must not retain a reference to the callback if |
Looks like we've resolved this with #40. Feel free to open a new issue if there's more focused discussion that remains. |
Moving this conversation here.
The text was updated successfully, but these errors were encountered: