Unable to extend JSX properties of already defined elements

[marvinhagemeister]

Bug Report

We've just release Preact Signals which are sort of an addon to both Preact and React that allow you to pass an observable object directly into JSX. As part of that we need to extend the existing JSX definitions to allow passing our observable type too, instead of just the existing types. But I'm unable todo so. I keep getting a type error.

🔎 Search Terms

JSX, declaration merging, React, Preact, Signals, IntrinsicElements

🕗 Version & Regression Information

• This is the behavior in every version I tried, and I reviewed the FAQ for entries about JSX + declaration merging

⏯ Playground Link

Playground link with relevant code

💻 Code

import * as React from "react"

// This and the JSX additions live in a library
interface Signal<T> {
    value: T
}

declare global {
    namespace JSX {
        interface IntrinsicElements {
            // Doesn't work
            input:
                | React.DetailedHTMLProps<React.InputHTMLAttributes<HTMLInputElement>,HTMLInputElement>
                | Signal<string>
        }
    }
}

// This represents app code the user writes
const foo: Signal<string> = {
    value: "foo"
}

const a = <input value={foo} />

🙁 Actual behavior

Passing a Signal as value errors with a type error

const foo = { value: "foo" };
const a = <input value={foo} />

🙂 Expected behavior

Passing a Signal as value should work

const foo = { value: "foo" };
const a = <input value={foo} />

[Andarist]

Note that you don’t have to use (and IMHO shouldn't) the global JSX namespace.

You could solve this by configuring jsxImportSource that would bring its own local JSX namespace that wouldn't pollute the global namespace. I see that you are already utilizing the local JSX namespace in Preact. I think that you should reexport manipulated JSXInternal together with runtime factories from your @preact/signals package and that people should point to that package in their tsconfig.json

[marvinhagemeister]

Yeah, in Preact itself we're using local JSX namespaces. Telling users to point to another package for JSX would work, but I'm worried that this approach breaks as soon as there is another package that also wants to extend the types. Today it's only Signals, but I could envision there being a native adapter for RxJS bindings for example. So we'd need a way to combine all of them.

[Andarist]

Telling users to point to another package for JSX would work, but I'm worried that this approach breaks as soon as there is another package that also wants to extend the types.

This is a valid concern but there is still probably a way to handle this through the composition of types. Note that this would at least reference the "composed" module somewhere and it would make it discoverable - instead of relying on implicit extensions. It also wouldn't require any new language constructs to be introduced - right now those interfaces just behave in the very same way the usual interface merging works but that, of course, doesn't fit your use case. This behavior can't change - so something new would have to be introduced to account for this.

The problem with interface merging, in general, is that it requires the full knowledge of the project to work correctly - so by nature, it slows down the project startup times and stuff. Just my 2 cents but explicit deps >>> implicit deps, for this very reason.

[RyanCavanaugh]

I'm worried that this approach breaks as soon as there is another package that also wants to extend the types.

It's not really clear how to make this scenario work in a generalizable way, except via the one already proposed upthread. If Signal2 also existed and also wants to make things work, what happens?

// In Signal
            input: TheReactDefinition | Signal<string>

// In Signal2
            input: TheReactDefinition | Signal2<string>

Is TS supposed to merge these two types using Union? Intersection? How/why? It's very unclear. Even if we had some keyword like merge so you could write input: merge | Signal<string> to refer to the property you're overwriting, then you run into declaration ordering problems because repeated utterances of merge in the same property slot could manifest in ways that have order-dependent nesting (e.g. input: Foo<merge>)

[DanielRosenwasser]

One other thing I would like to understand better is how Preact makes this work for React. Usually the question of "what should the types do here" can more-easily be answered by asking precisely how does this work at runtime?

[marvinhagemeister]

Is TS supposed to merge these two types using Union? Intersection? How/why? It's very unclear. Even if we had some keyword like merge so you could write input: merge | Signal to refer to the property you're overwriting, then you run into declaration ordering problems because repeated utterances of merge in the same property slot could manifest in ways that have order-dependent nesting (e.g. input: Foo)

This is a very good point and I hadn't thought about that when creating the issue initially. Agree with you that there is too much ambiguity and it can easily lead down into a rabbit hole. Sounds like explicitly composing the types in the consuming code base is the way to go.

One other thing I would like to understand better is how Preact makes this work for React. Usually the question of "what should the types do here" can more-easily be answered by asking precisely how does this work at runtime?

For Preact it's obviously a lot easier since we provide official ways to hook into the renderer. With React it's more tricky and involves hacking into internals. It's something I assume is next to impossible to derive types from for a type system as it's too buried behind a bunch of abstractions.