[labs/context] ContextProvider added after hostConnected does not work with ContextRoot

[kevinpschaaf]

Which package(s) are affected?

Context (@lit-labs/context)

Description

Attaching a ContextRoot is designed to allow a ContextProvider to come into existence after a consumer requests the data.

We have a test for this by registering an element that has a ContextProvider after the consumer is registered.

However, another use case is for a ContextProvider controller to be added lazily to an element, after it is already registered/connected. The key difference here is when hostConnected runs: When calling addController on an already connected element, hostConnected will be run synchronous to that call; when calling addController during an element construction, hostConnected will be called later, when the element's connectedCallback lifecycle runs.

This difference manifests in a bug due to the fact that the ContextProvider controller's context-request listener is added after addController runs:

lit/packages/labs/context/src/lib/controllers/context-provider.ts

Lines 46 to 54 in 00b8c56

	constructor(
	protected host: ReactiveElement,
	private context: T,
	initialValue?: ContextType<T>
	) {
	super(initialValue);
	this.host.addController(this);
	this.attachListeners();
	}

However during the hostConnected callback (which might be called synchronously to addController as mentioned above), we fire the context-provider that, when used in conjunction with ContextRoot, results in a context-request event being fired back to the provider:

lit/packages/labs/context/src/lib/controllers/context-provider.ts

Lines 76 to 79 in 00b8c56

	hostConnected(): void {
	// emit an event to signal a provider is available for this context
	this.host.dispatchEvent(new ContextProviderEvent(this.context));
	}

As such, the context-request event can be missed by the ContextProvider. The solution is hopefully as simple as flipping the addController() and attachListeners() lines.

Reproduction

https://lit.dev/playground/#gist=f709d0dc42035197bb2b4fc4ec65449a

Workaround

This workaround makes the above repro work as expectd:

  const hostConnected = ContextProvider.prototype.hostConnected;
  ContextProvider.prototype.hostConnected = async function() {
    await 0;
    hostConnected.call(this);
  }

Is this a regression?

No or unsure. This never worked, or I haven't tried before.

Affected versions

@lit-labs/context 0.1.3

Browser/OS/Node environment

Browser: any

[justinfagnani]

What's the use case for adding a ContextProvider lazily?

[kevinpschaaf]

@justinfagnani This is the bug I mentioned yesterday, added to your sprint. The solution is hopefully as simple as flipping the addController() and attachListeners() lines.

What's the use case for adding a ContextProvider lazily?

I was experimenting with fetching a JSON file containing values to provide as context. Because the keys aren't known at element construction time, it looped over the JSON keys after the fetch resolved and created ContextProviders.

[justinfagnani]

@kevinpschaaf I wonder if an even better solution here is to make ContextProvider able to listen for a dynamic set of multiple context keys. Then you could create one ContextProvider up front, and adjust what it provides after the fact.

[justinfagnani]

@nachoab what's your use case for lazy adding a ContextProvider?

[nachoab]

I was experimenting with contexts on a microservices frontend, in which lazily loaded packages could register providers on the app-shell.

[kevinpschaaf]

make ContextProvider able to listen for a dynamic set of multiple context keys

@justinfagnani We should explore that, but I think we should also just fix this, since it's really just a bug. The listeners should be setup before connecting it to the host which might start firing events. The fix is to just flip the addController() and attachListeners() lines.