Base types on generics, not global JSX namespace #6

remcohaszing · 2024-02-16T13:19:11Z

Initial checklist

I read the support docs
I read the contributing guide
I agree to follow the code of conduct
I searched issues and couldn’t find anything (or linked relevant results below)
If applicable, I’ve added docs and tests

Description of changes

For a while now, JSX frameworks don’t have to declare the global JSX namespace anymore. This is a significant improvement when using multiple JSX frameworks in the same project. React has deprecated the global JSX namespace. Preact doesn’t even define it anymore.

This change replaces the use of the JSX namespace with generics, to make it compatible with modern JSX frameworks. I also have an alternative branch locally which accepts only one generic, which is the type of the jsx function.

This removes the inference of props in custom components. I’m not sure what the impact is of that.

This could be considered a breaking change on the type level.

Closes #4

wooorm

When did this change? Which TS versions can(not) work without JSX?

types.d.ts

wooorm · 2024-02-16T15:37:23Z

package.json

@@ -108,6 +108,7 @@
    "ignoreFiles": [
      "example/**/*.js"
    ],
+    "ignoreNested": true,


Otherwise type-coverage reports nested any types on types that we don’t control, nor where we care at all about its impact. E.g. ReactElement.

wooorm · 2024-02-16T15:38:17Z

lib/index.js

 */

 /**
- * @typedef {JSX.Element | string | null | undefined} Child
+ * @template JSXElement
+ * @typedef {JSXElement | string | null | undefined} Child


template is sometimes defined before and sometimes after. Make this consistent.

And, can you add descriptions to them?

Yes, I first ran into some weird behaviour regarding order. Splitting into multiple block comments seems to have fixed everything.

Do you have a preference where they are added? I’ll add descriptions.

Yes, for some reason the templates are “per block” 🤷‍♂️

Ehh, consistency is most important. I think I recently liked before more, but not 100%, could’ve been the other way around!

wooorm · 2024-02-16T15:40:19Z

lib/index.js

 */

 /**
- * @typedef {JSX.Element | string | null | undefined} Child
+ * @template JSXElement


Adding required type parameters is a breaking change.
Is that worth it? Should there be defaults?

Also, all these types are documented in the readme but this PR does not change the docs.

The types of lib/index.js are private. It’s ok to make such changes here.

index.js exposes the public interface. The addition of required generics there is indeed a breaking change. Also the Components and ExtraProps types are removed. We’d better discuss whether or not this change is breaking based on those changes.

I suspect the breaking impact is low, but this is breaking indeed on the type level.

I posted here because it was the first, and Child is in Props.
As I noted somewhere else, these APIs are supposed to be used by people. They need to be documented in the readme and learned by humans.

Also, about what to discuss, see my last comment: #6 (comment).

wooorm · 2024-02-16T15:49:37Z

lib/index.js

 */

 /**
- * @typedef {JSX.Element | string | null | undefined} Child
+ * @template JSXElement
+ * @typedef {JSXElement | string | null | undefined} Child
 *   Child.


How useful is a Child<X> type if all it does is add X to an enum of 3 primitive values? Could it be replaced by X | Child?

It’s used several times. I think it’s ok to keep.

These types are exposed from this project. They are meant to be used and they are documented for humans to learn. They can be abused (someone could use it with any value that is unrelated to JSX). PrimitiveChild | MyJsxElement seems like a better API to me than Child<AnyCustomValue>

wooorm · 2024-02-16T15:51:12Z

lib/index.js

 *   Fragment symbol.
 * @property {Array<Parents>} ancestors
 *   Stack of parents.
- * @property {Partial<Components>} components
+ * @property {Record<string, JSXElementType>} components


It’s a bit sad that here we loose the info on blockquote meaning particular props and corresponding components though, right?

Yes, I find this unfortunate too. I’m not sure where this is used. The Components type doesn’t seem to be exposed via @mdx-js/mdx, so I think the impact of this in the end is very minimal.

This project is not used in MDX I believe. It’s a separate tool that can be used by anyone. It’s also used in react-markdown and in rehype-react. We’d loose support there too

You’re right. I thought it was. Only some types are used (for evaluate).

wooorm · 2024-02-16T16:16:24Z

I also have an alternative branch locally which accepts only one generic, which is the type of the jsx function.

that sounds pretty nice 🤔 why not?

remcohaszing · 2024-02-20T12:54:21Z

When did this change? Which TS versions can(not) work without JSX?

Only really old TypeScript versions can’t work without the global JSX namespace. The ${jsxPragma}.JSX namespace has been around for a while. I think it slightly predates the automatic runtime. But it took a long time for JSX frameworks to actually use it. Now it appears frameworks are moving towards doing so.

Although I recommend against using the global JSX namespace, for @types/mdx this was convenient. Thankfully there’s a workaround, for example for Preact (I need to confirm this and PR DefinitelyTyped to update the docs):

declare module 'mdx/types' {
  import { JSX } from 'preact/jsx-runtime'
}

I also have an alternative branch locally which accepts only one generic, which is the type of the jsx function.

that sounds pretty nice 🤔 why not?

It does sound nice! I ran into some JSDoc limitations though, that make it impossible to make this work without converting toJsxRuntime to TypeScript. Also it feels kind of weird to infer the development runtime types based on typeof production.jsx.

shellscape · 2024-02-20T14:32:54Z

I was about to open an issue requesting that the types (FunctionalComponent, etc) from ./components.js be exported by index.d.ts. Why are those being removed?

remcohaszing · 2024-02-20T14:55:57Z

Those types depend on the global JSX namespace, which is being deprecated / removed by some frameworks. I recommend using framework specific types (i.e. ReactElement, ElementType, FunctionComponent, etc. from @types/react. If you really need framework agnostic types, I suggest you use @types/mdx. Those types also take JSX.ElementType into account, wich the types removed in this PR do not.

Note that you only need framework agnostic JSX types if you write something where those types become part of the public interface of a library. You can likely use @types/react instead.

shellscape · 2024-02-20T14:59:27Z

Note that you only need framework agnostic JSX types if you write something where those types become part of the public interface of a library. You can likely use @types/react instead.

That's exactly what I'm doing. I'll have compat between React, Preact, and SolidJS when done.

remcohaszing · 2024-02-20T15:05:27Z

But will those JSX based types be part of the public interface?

Anyway, that should also be possible with the generics solution proposed in this PR.

shellscape · 2024-02-20T15:06:41Z

But will those JSX based types be part of the public interface?

Yes.

Anyway, that should also be possible with the generics solution proposed in this PR.

Would love to see an example using the changes in the PR

wooorm · 2024-02-27T10:11:20Z

${jsxPragma}.JSX

It sounds like we could use that registry here to type which components are allowed?
Why not use that?

toJsxRuntime to TypeScript

Curious to hear more about those limitations.

Also it feels kind of weird to infer the development runtime types based on typeof production.jsx.

a) we might be able to support both? b) many runtimes do jsxDEV = jsxs = jsx, and, I think there’s some sort of “contract” that they should yield the same things

For a while now, JSX frameworks don’t have to declare the global `JSX` namespace anymore. This is a significant improvement when using multiple JSX frameworks in the same project. React has deprecated the global `JSX` namespace. Preact doesn’t even define it anymore. This change replaces the use of the `JSX` namespace with generics, to make it compatible with modern JSX frameworks.

remcohaszing · 2024-03-01T16:55:34Z

${jsxPragma}.JSX

It sounds like we could use that registry here to type which components are allowed? Why not use that?

That was for the classic runtime, but even if we used that, we can’t access that. For the automatic runtime, we need the JSX namespace imported from ${jsxImportSource}/jsx-runtime.

toJsxRuntime to TypeScript

Curious to hear more about those limitations.

I did get it to work after all. Default type parameter values behave somewhat different in types in JSDoc. This was confusing.

wooorm · 2024-03-01T17:57:12Z

but even if we used that, we can’t access that.

I don’t understand. We can access it, but we can’t?

we need the JSX namespace imported [...]

Do you have an example of how that works? I don’t think I’ve seen Preact or so expose those things.

remcohaszing · 2024-03-18T12:55:09Z

but even if we used that, we can’t access that.

I don’t understand. We can access it, but we can’t?

It’s not possible to access types in a namespace from the typeof such a namespace. I.e. given a namespace…

namespace JSX {
  interface Element {}
}

… it is not possible to retrieve Element from typeof JSX. Nor is it possible to pass a namespace as a type parameter.

we need the JSX namespace imported [...]

Do you have an example of how that works? I don’t think I’ve seen Preact or so expose those things.

import { JSX } from 'react/jsx-runtime'
//       ^^^ We would need this value
//                   ^^^^^ But this part is dynamic in our case.

So we cannot infer anything from the JSX namespace. We were only able to do so previously, because frameworks were using the global JSX namespace.

remcohaszing · 2024-03-18T12:59:16Z

index.js

+ * @typedef {unknown} Components
+ * @typedef {unknown} ExtraProps


I suggest removing these stubs.

wooorm · 2024-03-18T14:20:18Z

We were only able to do so previously, because frameworks were using the global JSX namespace.

But like. Which frameworks have removed them? Are frameworks going to?
Because supporting components is quite nice here? Not sure I’d want to see it removed?

TS itself can validate whether x-y is a valid element and whether the props passed to it are fine. Why can’t we?

Take https://github.com/preactjs/preact/blob/a2c12f5a46a70b2b58517f5e14e731a77d6d64a3/test/ts/custom-elements.tsx#L3 for example.

I find this really hard to review: there are no sources cited for all of this.

remcohaszing · 2024-03-18T16:00:22Z

The global JSX namespace is removed by preact, solid-js, and vue. For react it’s deprecated.

Unfortunately demonstrating this doesn’t work well in a playground. Instead, you can try with these files:

// package.json
{
  "type": "module",
  "dependencies": {
    "preact": "^10.0.0",
    "solid-js": "^1.0.0",
    "typescript": "^5.0.0",
    "vue": "^3.0.0"
  }
}

// tsconfig.json
{
  "compilerOptions": {
    "module": "nodenext",
    "jsx": "react-jsx",
    "jsxImportSource": "preact",
    "noEmit": true
  }
}

// index.tsx
// These imports exist to show none of them register a global `JSX` namespace anymore.
import 'preact'
import 'solid-js'
import 'vue'
import {/* JSX */} from 'preact/jsx-runtime'
import {/* JSX */} from 'solid-js/jsx-runtime'
import {/* JSX */} from 'vue/jsx-runtime'

type JSXElement = JSX.Element

const element = <div>hello</div>

The type of <div> depends on the value of jsxImportSource in tsconfig.json. You can try this by changing jsxImportSource and Ctrl + clicking the <div> element in the code. The type of <div> and element is based on the JSX export of the ${jsxImportSource}/jsx-runtime module. TypeScript can use the information of the JSX namespace, because it’s something built into TypeScript. As a library we can’t.

For TypeScript exporting the JSX namespace is the only thing needed to support the JSX automatic runtime. At runtime, the jsx, jsxs, and Fragment values are needed. Some runtimes export them, but not all. Perhaps the current state is best viewed as a compatibility table.

Feature	`@types/react`	`preact`	`solid-js`	`vue`
Global `JSX` namespace¹	⚠️ deprecated	❌	❌	❌
Export `JSX` namespace²	✔️	✔️	✔️	✔️
Export `jsx`	⚠️ ³	✔️	❌	✔️
Export `jsxs`	⚠️ ³	✔️	❌	✔️
Export `Fragment`	✔️	✔️	❌	✔️

So basically the current state is:

hast-util-to-jsx-runtime depends on a feature that’s deprecated in @types/react and unsupported by other frameworks.
The dependency on this feature means it’s a type error unless @types/react is used.
React is the worst framework to represent the current state, because it’s the most loose.
All frameworks listed support the JSX automatic runtime, but we can’t use the information they provide for that.
It could be possible to infer props for preact and vue, but it requires more types gymnastics for little benefit IMO. And TBH getting to the current state took me more time and energy than I had hoped.
Projects that use this library for one specific runtime, such as react-markdown, can easily specify types based on the specific JSX framework, without using the types provided by hast-util-to-jsx-runtime.

Currently supported by hast-util-to-jsx-runtime ↩
Used by TypeScript. Not dynamicallyt by libraries such as hast-util-to-jsx-runtime ↩
Exported, but very loose type with no possibility of inferring the types of props ↩ ↩²

wooorm · 2024-03-19T10:32:52Z

The global JSX namespace is removed by preact, solid-js, and vue

Do you have references?

The docs still state that JSX is fine: https://www.typescriptlang.org/docs/handbook/jsx.html#intrinsic-elements.
This seems like a decision that affects React on DT, instead of something that affects all TS users. Otherwise, there would be docs changes to TS too right? Or references issues on TS repos?

React is the worst framework to represent the current state, because it’s the most loose.

🤔 The current existing JSX namespace is pretty good? 🤔 You mean that the types of the jsx function are bad?

All frameworks listed support the JSX automatic runtime, but we can’t use the information they provide for that.

We could if folks pass the JSX types from their /jsx-runtime in?

OK, I think I understand the concerns. The JSX global is going away. Fine. That solves some things for some users.
But. There is no good alternative. The types of jsx and such are often bad. We can’t really type components well.
At that point. Why even do this PR? We could return unknown. Or indeed, require folks to pass in several JSX types.

remcohaszing · 2024-03-21T11:12:14Z

The global JSX namespace is removed by preact, solid-js, and vue

Do you have references?

This PR deprecates usage in @types/react DefinitelyTyped/DefinitelyTyped#64464. There must be a commit somewhere that removes it for Preact, but I can’t find it quickly.

The docs still state that JSX is fine: https://www.typescriptlang.org/docs/handbook/jsx.html#intrinsic-elements. This seems like a decision that affects React on DT, instead of something that affects all TS users. Otherwise, there would be docs changes to TS too right? Or references issues on TS repos?

The docs don’t mention where the JSX must be specified. This used to be global, later support was added for ${jsxPragma}.JSX, e.g. React.JSX / React.createElement.JSX, but it took a long time for frameworks to implement this. This was convenient for @types/mdx. Later the automatic runtime was added.

I don’t even see a mention of added support for ${jsxPragma}.JSX in any release notes. I guess this is because it doesn’t affect most users, only authors of JSX frameworks.

React is the worst framework to represent the current state, because it’s the most loose.

🤔 The current existing JSX namespace is pretty good? 🤔 You mean that the types of the jsx function are bad?

I mean this in the sense that using Preact or Vue in tests would have shown the problem being solved here.

All frameworks listed support the JSX automatic runtime, but we can’t use the information they provide for that.

We could if folks pass the JSX types from their /jsx-runtime in?

Yes, that’s the intention of this PR.

OK, I think I understand the concerns. The JSX global is going away. Fine. That solves some things for some users. But. There is no good alternative. The types of jsx and such are often bad. We can’t really type components well. At that point. Why even do this PR? We could return unknown. Or indeed, require folks to pass in several JSX types.

What do you mean by We could return unknown. Or indeed, require folks to pass in several JSX types.? As I see it, that’s what this PR does. It infers the types based on generics.

wooorm · 2024-03-21T11:25:39Z

This PR deprecates usage in @types/react DefinitelyTyped/DefinitelyTyped#64464. There must be a commit somewhere that removes it for Preact, but I can’t find it quickly.

Yeah I know the one where DT maintainers change something in the React types. But there is no discussion about it. You make a lot of statements about how things are and that Preact/Vue/etc changed something. But there’s nothing backing that up yet.

This used to be global

Right, so I miss sources for such statements. There’s only a change in DT for React. Nothing on the TS part. Or other frameworks.
And, these docs, they show the global JSX being used right? Otherwise it would explain something along the lines of “for React, use React.JSX, for vue, do vue.JSX”, etc.

Yes, that’s the intention of this PR.

As I see it, that’s what this PR does. It infers the types based on generics.

From where does it infer? From the jsx function right? But, we have established that practically, current frameworks type those functions badly.
What if we either: a) require people to pass types in from React.JSX or equivalent (so React.JSX.Element, React.JSX.ElementClass, etc), b) bail and return unknown.

wooorm · 2024-10-11T15:30:53Z

I think this PR removes too many useful features.

JSX is great. JSX is used in millions of files. It’s super useful to type components and return values. See also #9 (comment).

I would be open to discussing bare minimal components, or returning unknown if JSX.Element is not defined, stuff like that, to not error.

remcohaszing added 🦋 type/enhancement This is great to have ☂️ area/types This affects typings 💬 type/discussion This is a request for comments 🤞 phase/open Post is being triaged manually labels Feb 16, 2024

remcohaszing changed the title ~~Generics~~ Base types on generics, not global JSX namespace Feb 16, 2024

wooorm reviewed Feb 16, 2024

View reviewed changes

remcohaszing force-pushed the generics branch from 084384b to e661865 Compare February 20, 2024 12:59

wooorm mentioned this pull request Feb 27, 2024

Change the jsx callback type type to any #4

Closed

5 tasks

remcohaszing added 4 commits March 1, 2024 16:25

Add test components param type annotations

c1e1119

Move all @template tags on top

ec61793

Accept the JSX function type as a generic

fde3379

remcohaszing force-pushed the generics branch from e661865 to fde3379 Compare March 1, 2024 16:54

remcohaszing added 2 commits March 1, 2024 18:07

Add missing inline docs

c4ed721

Fix type-coverage issues

79ad7d1

remcohaszing added 2 commits March 18, 2024 13:44

Fix type of self

44edfb9

Revert unrelated changes

9e01023

remcohaszing commented Mar 18, 2024

View reviewed changes

wooorm mentioned this pull request Apr 4, 2024

Global JSX to local JSX w/ generics #9

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Base types on generics, not global JSX namespace #6

Base types on generics, not global JSX namespace #6

remcohaszing commented Feb 16, 2024 •

edited

Loading

wooorm left a comment

wooorm Feb 16, 2024

remcohaszing Feb 20, 2024

wooorm Feb 16, 2024

remcohaszing Feb 16, 2024

wooorm Feb 19, 2024

wooorm Feb 16, 2024

remcohaszing Feb 20, 2024

wooorm Feb 20, 2024

wooorm Feb 16, 2024

remcohaszing Feb 20, 2024

wooorm Feb 20, 2024

wooorm Feb 16, 2024

remcohaszing Feb 20, 2024

wooorm Feb 20, 2024 •

edited

Loading

remcohaszing Mar 1, 2024

wooorm commented Feb 16, 2024

remcohaszing commented Feb 20, 2024

shellscape commented Feb 20, 2024

remcohaszing commented Feb 20, 2024

shellscape commented Feb 20, 2024

remcohaszing commented Feb 20, 2024

shellscape commented Feb 20, 2024

wooorm commented Feb 27, 2024

remcohaszing commented Mar 1, 2024

wooorm commented Mar 1, 2024

remcohaszing commented Mar 18, 2024

remcohaszing Mar 18, 2024

wooorm commented Mar 18, 2024

remcohaszing commented Mar 18, 2024

wooorm commented Mar 19, 2024 •

edited

Loading

remcohaszing commented Mar 21, 2024

wooorm commented Mar 21, 2024 •

edited

Loading

wooorm commented Oct 11, 2024

		* @typedef {unknown} Components
		* @typedef {unknown} ExtraProps

Base types on generics, not global JSX namespace #6

Are you sure you want to change the base?

Base types on generics, not global JSX namespace #6

Conversation

remcohaszing commented Feb 16, 2024 • edited Loading

Initial checklist

Description of changes

wooorm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wooorm Feb 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wooorm commented Feb 16, 2024

remcohaszing commented Feb 20, 2024

shellscape commented Feb 20, 2024

remcohaszing commented Feb 20, 2024

shellscape commented Feb 20, 2024

remcohaszing commented Feb 20, 2024

shellscape commented Feb 20, 2024

wooorm commented Feb 27, 2024

remcohaszing commented Mar 1, 2024

wooorm commented Mar 1, 2024

remcohaszing commented Mar 18, 2024

Choose a reason for hiding this comment

wooorm commented Mar 18, 2024

remcohaszing commented Mar 18, 2024

Footnotes

wooorm commented Mar 19, 2024 • edited Loading

remcohaszing commented Mar 21, 2024

wooorm commented Mar 21, 2024 • edited Loading

wooorm commented Oct 11, 2024

remcohaszing commented Feb 16, 2024 •

edited

Loading

wooorm Feb 20, 2024 •

edited

Loading

wooorm commented Mar 19, 2024 •

edited

Loading

wooorm commented Mar 21, 2024 •

edited

Loading