Conversation
|
Working on hydration tests, I know that is missing |
|
We are not, this provides the API required for state management but doesn't implement it.
It's something else
What's different?
|
packages/@lwc/integration-karma/test/component/context/lifecycle/index.spec.js
Outdated
Show resolved
Hide resolved
|
This is looking great @jhefferman-sfdc! Definitely moving in the right direction. |
packages/@lwc/integration-karma/scripts/karma-plugins/hydration-tests.js
Outdated
Show resolved
Hide resolved
| contextVariety: V, | ||
| providedContextSignal: Signal<unknown> | ||
| ): void { | ||
| // registerContextProvider is called one time when the component is first provided context. |
There was a problem hiding this comment.
Is this always true? For example, what if we instantiate a state manager (which implements ContextConnector) and attach it as a prop to two separate components?
I think we decided that "the latest context wins" in scenarios like that. But let's make sure the comments reflect that potential use-case, and make sure the implementation/tests cover that use case, as well.
There was a problem hiding this comment.
Is this always true? For example, what if we instantiate a state manager (which implements ContextConnector) and attach it as a prop to two separate components?
IIUC, yes? A state manager doesn't instantiate ContextConnector but rather consumes it? This is based on the the experimental impl here.
There is one listener per contextful component (aka a component we know to have associated context).
- The consumer event is fired and bubbles up, searching for the required context variety
- This one listener on the provider calls back to the consumer and says "hey, is any of my context what you are looking for?"
- If it there is a match, bubbling stops and if no match, the search continues
Please correct any misunderstandings I may have.
I think we decided that "the latest context wins" in scenarios like that. But let's make sure the comments reflect that potential use-case, and make sure the implementation/tests cover that use case, as well.
Yes that scenario is encapsulated below, here. I changed it to a warn to align with Signals implementation.
| } | ||
| } | ||
|
|
||
| export function disconnectContext(vm: VM) { |
There was a problem hiding this comment.
The exposed functions look good. Let's just add docs, and I think they're finished.
There was a problem hiding this comment.
These are not exposed via engine-server/dom - IIUC:
ContextConnector(previously,ContextRuntimeAdaptor) is the exposed API- connectContext (this function) is called once per component lifecycle, privately by vm.ts here.
- connectContext purpose is to make any component-associated context providable and consumable (via ContextConnector).
- I documented
ContextConnectorand the other API here. - I also exported it as a public interface here is the public API?
- This aligns with prior ContextRuntimeAdaptor
| connectWireAdapters(vm); | ||
| } | ||
|
|
||
| if (lwcRuntimeFlags.ENABLE_EXPERIMENTAL_SIGNALS) { |
There was a problem hiding this comment.
@divmain I protected the new context implementation with the same ENABLE_EXPERIMENTAL_SIGNALS for now, in case rollback is required? Could change to separate gate but I figured the features are closely related?
There was a problem hiding this comment.
Re-using the flag makes sense to me. They're closely entangled, as you say.
packages/@lwc/integration-karma/test-hydration/context/x/contextManager/contextManager.js
Show resolved
Hide resolved
| connectWireAdapters(vm); | ||
| } | ||
|
|
||
| if (lwcRuntimeFlags.ENABLE_EXPERIMENTAL_SIGNALS) { |
There was a problem hiding this comment.
Re-using the flag makes sense to me. They're closely entangled, as you say.
3 participants
Details
Expose context api for CSR/SSRv1 for state management support
See: https://github.com/salesforce/lightning-labs/tree/main/packages/%40lwc/state for context on the context :)
Does this pull request introduce a breaking change?
Does this pull request introduce an observable change?
GUS work item
W-18293936