Skip to content
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

[lit/context] Declarative context registry #4601

Open
1 task done
jun-sheaf opened this issue Mar 27, 2024 · 9 comments
Open
1 task done

[lit/context] Declarative context registry #4601

jun-sheaf opened this issue Mar 27, 2024 · 9 comments

Comments

@jun-sheaf
Copy link

jun-sheaf commented Mar 27, 2024

Should this be an RFC?

  • This is not a substantial change

Which package is this a feature request for?

Context (@lit/context)

Description

Currently, the context API uses createContext to create a branded type using a cast. This is okay, but there is redundancy. If @lit/context exposed their own ContextRegistry interface for users to extend with their own symbol, the redundancy disappears. We also have stronger type guarantees and we can just pass around unique symbols as defined in TS.

For example, consider the following example:

const RouterContextKey = Symbol("RouterContextKey");
export const routerContext = createContext<
  RouterContext,
  typeof RouterContextKey
>(RouterContextKey)

where RouterContext is some value we are passing around.

Note there is a lot of redundancy in createContext because partial specialization (i.e. createContext<RouterContext>(RouterContextKey)) is not possible. With the proposed feature, this would change into

const RouterContextKey = Symbol("RouterContextKey");

declare module "@lit/context" {
  interface ContextMap {
    [RouterContextKey]: RouterContext;
  }
}

Note that this can all be implemented without breaking changes using overloads.

Alternatives and Workarounds

Just use the current API.

@augustjk
Copy link
Member

Oohhh excellent point! Thanks for raising this. I didn't realize createContext's typing had this problem.

Related TS issues:
microsoft/TypeScript#10571
microsoft/TypeScript#26242

I've naively just been using createContext<ContextValue>(key) without providing the second type arg which loses the type of the context key as it, instead of getting inferred, gets defaulted to unknown. Though I'm not sure how important that is in practice. I mostly cared that the context value type is present for the provider/consumer, and haven't come across a situation where the type of the context key mattered.

Nevertheless, it appears in the code that the intention was to allow stricter context type that incorporates the type of the key.

I don't know if the proposed ContextMap interface/registry is a more ergonomic solution though.

In the wild, creating a currying function which takes the specified type and returns a function that infers the rest seem to be the workaround people have adopted in the wild e.g. zustand, or similarly a static method that returns the type narrowed implementation e.g. react-redux.

Using zustand's approach would look something like this:

type CreateContext = {
  <ValueType>(): <K = unknown>(key: K) => Context<K, ValueType>;
  <ValueType, K = unknown>(key: K): Context<K, ValueType>;
};

function createContextImpl<ValueType, K = unknown>(key: K) {
  return key as Context<K, ValueType>;
}

export const createContext = (<K>(key?: K) =>
  key !== undefined
    ? createContextImpl(key)
    : createContextImpl) as CreateContext;

which allows

type Foo = {foo: string};
const key = Symbol('foo');
const ctx = createContext<Foo>()(key); // notice the extra `()`
//    ^? Context<typeof key, Foo>

@jun-sheaf
Copy link
Author

jun-sheaf commented Mar 28, 2024

I'm aware that currying is possible, but using a type registry would be better since we can just pass around the symbol instead of some branded object. It's more hassle to understand and as a bonus Lit users would be familiar with the pattern since we do

declare global {
    interface HTMLElementTagNameMap {
       ...
    }
}

Also, using currying is more of a hack than an intended flow. I suspect if people knew more about module augmentation, this solution may be more prevalent. Then again, I don't know what zustand is used for so perhaps their use case warrants currying.

@augustjk
Copy link
Member

That's a fair point. We have a clear relation of context key to context value so a registry makes sense.

The examples above are for state managers that take a custom user state type arg and infer the rest, without a clear mapping relation, and therefore resorting to the currying solution.

@augustjk
Copy link
Member

@jun-sheaf I am curious though, have you come across a situation where this kind of strict association of key type to the context object was needed? Maybe the problem is that we didn't need the key type to be part of the branded context to begin with. As long as the branded context includes the value type, what's the benefit of passing around a bare symbol versus a branded one?

@justinfagnani
Copy link
Collaborator

I don't think createContext actually requires that boilerplate. It's an identity function that just adds a brand property to carry the value type. The key type is inferred from the argument, so you can write the key inline, like so:

export const routerContext = createContext<RouterContext>(Symbol('RouterContextKey'));

@jun-sheaf
Copy link
Author

It could very well be the case that we don't really need to know the key since all we care about is it carrying the correct value type. I liken a registry to a list of dependencies. We use a custom version that has all the symbols in one file and declared in the registry. It's a good place to go for documentation, but it could also just be done using branded types.

Perhaps we can just remove the key type from the context.

@justinfagnani
Copy link
Collaborator

The key type helps correctly type the identify function aspect of createContext() so I don't think we can remove it.

But removing it wouldn't lessen any boilerplate either, you would still write:

export const routerContext = createContext<RouterContext>(Symbol('RouterContextKey'));

^ the key type is completely inferred there and doesn't need to be written.

@jun-sheaf
Copy link
Author

jun-sheaf commented Mar 28, 2024

I don't think createContext actually requires that boilerplate. It's an identity function that just adds a brand property to carry the value type. The key type is inferred from the argument, so you can write the key inline, like so:

export const routerContext = createContext<RouterContext>(Symbol('RouterContextKey'));

The key type isn't inferred from the argument. It will be unknown.

However I think @augustjk and I agree that the key type is completely unnecessary.

@jun-sheaf
Copy link
Author

jun-sheaf commented Mar 28, 2024

The key type helps correctly type the identify function aspect of createContext() so I don't think we can remove it.

Could you elaborate? Are you talking about for comparison of two contexts? The question there is what reason do you have to compare two contexts? If you could come up with a reason, then this would strengthen the registry case since it would imply users want a strong link between keys and values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

3 participants