sveltejs / svelte

web development for the rest of us
https://svelte.dev
MIT License
80.15k stars 4.26k forks source link

Use ContextKey<T> to type setContext, getContext and hasContext #8941

Open olehmisar opened 1 year ago

olehmisar commented 1 year ago

Describe the problem

Currently getContext returns unknown if no explicit type is provided.

image

Describe the proposed solution

Taking inspiration from svelte-typed-context, getContext can accept an InjectionKey<T>ContextKey<T> and return T:

image

This will improve type safety and can be done in a backwards compatible way.

The implementation would be just to add a new signature to getContext, setContext and hasContext

export interface ContextKey<T = unknown> { }
export function setContext<T>(key: ContextKey<T>, context: T): void
// getContext ...
// hasContext ...
// check for full implementation here:
// https://github.com/KamenKolev/svelte-typed-context/blob/master/index.ts
// (published npm package does not include `hasContext`, so install only from github)

Alternatives considered

-

Importance

nice to have

Search terms

typed context inject provide

brunnerh commented 1 year ago

Maybe just call it ContextKey, though?

olehmisar commented 1 year ago

@dummdidumm i just checked that adding a new function signature to {set,get,has}Context while preserving backwards compatibility is impossible. It's because the key argument is of type unknown and the new signature will not be typesafe because of that. If this feature is planned for the future, I think it's a great candidate for a new major release because it will be a breaking change.

oscarhermoso commented 9 months ago

You can add below snippet to src/app.d.ts and override the existing types :slightly_smiling_face:

type Context = {
    theme: Writable<string | null>; // for example
};

declare module 'svelte' {
    export function getContext<T>(key: T extends keyof Context ? T : never): Context[T];

    export function setContext<T>(
        key: T extends keyof Context ? T : never,
        context: Context[T],
    ): void;
}
NickClark commented 9 months ago

To extend the ideas mentioned above, my preferred app.d.ts flavor is:

declare module "svelte" {
  export interface ContextKey<T = unknown> {} // eslint-disable-line @typescript-eslint/no-unused-vars

  export function getContext<T>(key: ContextKey<T>): T;
  export function setContext<T>(key: ContextKey<T>, value: T): void;
  export function hasContext<T>(key: ContextKey<T>): boolean;
}

Then you can define your key as simply as:

export const TabContext: ContextKey<Tab[]> = {};
NickClark commented 9 months ago

The only real issue with the solution above, is that setContext ends up being an overload, so when trying to set the context value it falls back to the generic definition and won't autocomplete.

setContext(MyContextKey, { /* Typescript won't autocomplete this */ });

Anyone know of a way to override instead of extend?

Carlos-err406 commented 7 months ago

i also tried the approaches above but cant get autocompletion on the value of set context, only on the key here is my own flavor, importing Context from $lib/types

declare module 'svelte' {
    export function getContext<T extends keyof Context, K extends Context[T]>(key: T): K;
    export function setContext<T extends keyof Context, K extends Context[T]>(
        key: T,
        context: K
    ): void;
}
rChaoz commented 3 months ago

To maintain backwards compatibility, how about a new syntax:

// wherever.ts
export const MyContext = Context<string>()
// Parent.svelte
import MyContext from '...'

MyContext.set("123")
// Child.svelte
const value = MyContext.get() // returns string

The implementation for such a context class would be insanely simple and even allow for default values:

class Context<T> {
    key: Symbol
    default: T

    constructor(default?: T) {
        this.key = Symbol()
        this.default = default
    }

    get() {
        // shouldn't return the default value if parent did Context.set(null)
        // maybe a new isContextSet(key) function would be needed?
        return (getContext(this.key) ?? this.default) as T
    }

    set(value: T) {
        setContext(this.key, value)
    }
}

Or, could be implemented as a function instead. This is my current solution, and I'd love to see a similar official solution:

export function createContext<T>(default?: T) {
    const key = Symbol()
    return {
        get value() {
            return (getContext(key) ?? default) as T
        }
        set value(new: T) {
            setContext(key, value)
        }
    }
}
ryanatkn commented 2 months ago

Here's a version that uses a function with overloaded signatures to infer types based on the presence of a fallback argument:

export function createContext<T>(fallback: () => T): {
    get: () => T;
    set: (value?: T) => T;
};
export function createContext<T>(): {
    get: (errorMessage?: string) => T;
    maybeGet: () => T | undefined;
    set: (value: T) => T;
};
export function createContext<T>(fallback?: () => T): {
    get: (errorMessage?: string) => T;
    maybeGet: () => T | undefined;
    set: (value?: T) => T;
} {
    const key = Symbol();
    function maybeGet() {
        const value: T | undefined = getContext(key);
        return value === undefined ? fallback?.() : value; // careful with `null`
    };
    return {
        get: (errorMessage?: string) => {
            const value = maybeGet();
            // runtime check because this is not typesafe, types can't know what's in context
            if (value === undefined) throw Error(errorMessage ?? 'context value is not set');
            return value;
        },
        maybeGet,
        // this is typesafe, so no runtime check, but it could be added, maybe just in `DEV`:
        set: (value: T | undefined = fallback?.()) => setContext(key, value)!,
    };
}
  1. with a fallback, get never throws, the argument to set is optional, get accepts no errorMessage, and maybeGet is omitted via types (could be actually omitted but this keeps it monomorphic and it's being used internally so it must exist as written where the goal is minimal code size)
  2. without a fallback, get throws on missing values, the argument to set is required, get accepts an optional errorMessage, and an additional function maybeGet is included that can return undefined

The fallback could be in an options object if other initial arguments were added, like for better errors.

The fallback may be confusing two different concerns for getting and setting. It could be split into two or take a boolean arg indicating when it's being called, but this may be needlessly complex.

Instead of maybeGet, the get function could take an optional/required argument, but I didn't think that API reads as clearly when used. They could also be swapped for get and getRequired/getOrThrow or something.

An unavoidable detail is that you need to specify the type if there's no fallback, e.g. createContext<SomeType>().

I'm curious about the tradeoffs between get/set functions and the .value getter/setter described above by @rChaoz. This implementation accepts an optional errorMessage to get when there's no fallback, which I find slightly useful for 1-liner usage and doesn't work with a getter. It also has an extra maybeGet function, which would be inconsistent alongside a getter. I also think I prefer the explicit function calls because set has the side effect, and I like being able to pass around the functions without the context wrapper in rare cases for composability, but thunks can do the same with a tiny bit more code.

The createContext helper could be implemented with a class if that was better for performance, but this seems simpler and mirrors the store code.

The class-based versions I could think of would either still require a createContext helper with overloaded function signatures, or push the complexity of 2 classes onto the user. I couldn't find a way for a single class implementation to infer the return type of get and argument type of set based on the constructor args. I don't think you can overload a class' whole type signature, just individual methods, which I couldn't make work.

I also don't see a reason to make key an option but maybe for debugging/tooling?

I originally wrote this comment with this worse version that forces you to declare if it's optional up front, and therefore needs no `maybeGet` function: ```ts export function createContext(options: {fallback: () => T}): { get: () => T; set: (value?: T) => T; }; export function createContext(options: {optional: true}): { get: () => T | undefined; set: (value: T) => T; }; export function createContext(options?: {optional?: false}): { get: () => T; set: (value: T) => T; }; export function createContext(options?: {fallback?: () => T; optional?: boolean}): { get: () => T | undefined; set: (value?: T) => T; } { const fallback = options?.fallback; const optional = options?.optional; const key = Symbol(); return { get: () => { const got: T | undefined = getContext(key); const value = got === undefined ? fallback?.() : got; // careful with `null` if (!optional && value === undefined) { // runtime check because this is not typesafe, can't know the tree above throw Error('context value is not set'); } return value; }, // this is typesafe, so no runtime check: set: (value: T | undefined = fallback?.()) => setContext(key, value)!, }; } ``` 1. with a `fallback`, the return type of `get` is always defined, and the argument to `set` is optional 2. without a `fallback` and with `optional: true`, the return type of `get` may be `undefined`, and the argument to `set` is required 3. without a `fallback` or `optional: true`, the return type of `get` is always defined because it throws if not, the argument to `set` is required, and `options` is optional The major limitation with this old version is that the context's optionality is defined at creation, rather than letting consumers decide with each `get` if they want to automatically throw on missing values when no fallback is provided.