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.

Sign up for GitHub

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

Jump to bottom

Fix the JSX runtime types in RunOptions #2465

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix the JSX runtime types in RunOptions #2465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions docs/_asset/editor.jsx
View file
Open in desktop
Expand Up @@ -204,9 +204,7 @@ function Playground() {
/** @type {MDXModule} */
const result = await run(String(file), {
Fragment,
// @ts-expect-error: to do: fix in `hast-util-to-jsx-runtime`.
jsx,
// @ts-expect-error: to do: fix in `hast-util-to-jsx-runtime`.
jsxs,
baseUrl: window.location.href
})
Expand Down
9 changes: 1 addition & 8 deletions docs/migrating/v3.mdx
View file
Open in desktop
Expand Up @@ -52,16 +52,9 @@ You will get a runtime error if these features are used in MDX without
If you passed the `useDynamicImport` option before, remove it, the behavior
is now the default.

If you use `react/jsx-runtime`, you might get a TypeScript error (such as
`Property 'Fragment' is missing in type`), because it is typed incorrectly.
To remediate this, do:

```tsx
import {type Fragment, type Jsx, run} from '@mdx-js/mdx'
import * as runtime_ from 'react/jsx-runtime'

// @ts-expect-error: the automatic react runtime is untyped.
const runtime: {Fragment: Fragment; jsx: Jsx; jsxs: Jsx} = runtime_
import * as runtime from 'react/jsx-runtime'

const result = await run('# hi', {...runtime, baseUrl: import.meta.url})
```
Expand Down
7 changes: 5 additions & 2 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions packages/mdx/index.js
View file
Open in desktop
@@ -1,7 +1,4 @@
/**
* @typedef {import('./lib/util/resolve-evaluate-options.js').Fragment} Fragment
* @typedef {import('./lib/util/resolve-evaluate-options.js').Jsx} Jsx
* @typedef {import('./lib/util/resolve-evaluate-options.js').JsxDev} JsxDev
* @typedef {import('./lib/util/resolve-evaluate-options.js').UseMdxComponents} UseMdxComponents
* @typedef {import('./lib/compile.js').CompileOptions} CompileOptions
* @typedef {import('./lib/core.js').ProcessorOptions} ProcessorOptions
Expand Down
11 changes: 4 additions & 7 deletions packages/mdx/lib/util/resolve-evaluate-options.js
View file
Open in desktop
@@ -1,7 +1,4 @@
/**
* @typedef {import('hast-util-to-jsx-runtime').Fragment} Fragment
* @typedef {import('hast-util-to-jsx-runtime').Jsx} Jsx
* @typedef {import('hast-util-to-jsx-runtime').JsxDev} JsxDev
* @typedef {import('mdx/types.js').MDXComponents} Components
* @typedef {import('../compile.js').CompileOptions} CompileOptions
*/
Expand Down Expand Up @@ -29,13 +26,13 @@
* this option can also be given at compile time in `CompileOptions`;
* you should pass this (likely at runtime), as you might get runtime errors
* when using `import.meta.url` / `import` / `export … from ` otherwise.
* @property {Fragment} Fragment
* @property {unknown} Fragment
* Symbol to use for fragments (**required**).
* @property {Jsx | null | undefined} [jsx]
* @property {unknown} [jsx]
* Function to generate an element with static children in production mode.
* @property {JsxDev | null | undefined} [jsxDEV]
* @property {unknown} [jsxDEV]
* Function to generate an element in development mode.
* @property {Jsx | null | undefined} [jsxs]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we at least use something like Function?
We could also improve the hast-util-to-jsx-runtime types?
Fixing the type error is nice, but removing all type errors around this, not so sure?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use Function, or better: (type: any, props: any) => any.

We should improve hast-util-to-jsx-runtime, but personally I think this type isn’t worth having an extra dependency regardless.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or better: (type: any, props: any) => any.

Right, and even better would be an even more correct type. We do know a lot about props and key. And at that point, it’s nice to have the more complex type defined and tested in a single place?

* @property {unknown} [jsxs]
* Function to generate an element with dynamic children in production mode.
* @property {UseMdxComponents | null | undefined} [useMDXComponents]
* Function to get components from context.
Expand Down
1 change: 0 additions & 1 deletion packages/mdx/package.json
View file
Open in desktop
Expand Up @@ -54,7 +54,6 @@
"estree-util-to-js": "^2.0.0",
"estree-walker": "^3.0.0",
"hast-util-to-estree": "^3.0.0",
"hast-util-to-jsx-runtime": "^2.0.0",
"markdown-extensions": "^2.0.0",
"periscopic": "^3.0.0",
"remark-mdx": "^3.0.0",
Expand Down
17 changes: 2 additions & 15 deletions packages/mdx/test/evaluate.js
View file
Open in desktop
@@ -1,25 +1,12 @@
/**
* @typedef {import('@mdx-js/mdx').Fragment} Fragment
* @typedef {import('@mdx-js/mdx').Jsx} Jsx
* @typedef {import('@mdx-js/mdx').JsxDev} JsxDev
*/

import assert from 'node:assert/strict'
import {test} from 'node:test'
import {evaluate, evaluateSync, compile} from '@mdx-js/mdx'
import * as provider from '@mdx-js/react'
import {renderToStaticMarkup} from 'react-dom/server'
import * as runtime_ from 'react/jsx-runtime'
import * as devRuntime_ from 'react/jsx-dev-runtime'
import * as runtime from 'react/jsx-runtime'
import * as developmentRuntime from 'react/jsx-dev-runtime'
import React from 'react'

/** @type {{Fragment: Fragment, jsx: Jsx, jsxs: Jsx}} */
// @ts-expect-error: the automatic react runtime is untyped.
const runtime = runtime_
/** @type {{Fragment: Fragment, jsxDEV: JsxDev}} */
// @ts-expect-error: the automatic dev react runtime is untyped.
const developmentRuntime = devRuntime_

test('@mdx-js/mdx: evaluate', async function (t) {
await t.test('should throw on missing `Fragment`', async function () {
try {
Expand Down
11 changes: 1 addition & 10 deletions packages/preact/test/index.jsx
View file
Open in desktop
@@ -1,21 +1,12 @@
/* @jsxRuntime automatic @jsxImportSource preact */

/**
* @typedef {import('@mdx-js/mdx').Fragment} Fragment
* @typedef {import('@mdx-js/mdx').Jsx} Jsx
*/

import assert from 'node:assert/strict'
import {test} from 'node:test'
import {evaluate} from '@mdx-js/mdx'
import {MDXProvider, useMDXComponents} from '@mdx-js/preact'
import * as runtime_ from 'preact/jsx-runtime'
import * as runtime from 'preact/jsx-runtime'
import {render} from 'preact-render-to-string'

const runtime = /** @type {{Fragment: Fragment, jsx: Jsx, jsxs: Jsx}} */ (
runtime_
)

test('@mdx-js/preact', async function (t) {
await t.test('should expose the public api', async function () {
assert.deepEqual(Object.keys(await import('@mdx-js/preact')).sort(), [
Expand Down
11 changes: 1 addition & 10 deletions packages/react/test/index.jsx
View file
Open in desktop
@@ -1,20 +1,11 @@
/**
* @typedef {import('@mdx-js/mdx').Fragment} Fragment
* @typedef {import('@mdx-js/mdx').Jsx} Jsx
*/

import assert from 'node:assert/strict'
import {test} from 'node:test'
import {evaluate} from '@mdx-js/mdx'
import {MDXProvider, useMDXComponents} from '@mdx-js/react'
import React from 'react'
import * as runtime_ from 'react/jsx-runtime'
import * as runtime from 'react/jsx-runtime'
import {renderToString} from 'react-dom/server'

const runtime = /** @type {{Fragment: Fragment, jsx: Jsx, jsxs: Jsx}} */ (
/** @type {unknown} */ (runtime_)
)

test('@mdx-js/react', async function (t) {
await t.test('should expose the public api', async function () {
assert.deepEqual(Object.keys(await import('@mdx-js/preact')).sort(), [
Expand Down
8 changes: 1 addition & 7 deletions packages/vue/test/index.js
View file
Open in desktop
@@ -1,6 +1,4 @@
/**
* @typedef {import('@mdx-js/mdx').Fragment} Fragment
* @typedef {import('@mdx-js/mdx').Jsx} Jsx
* @typedef {import('mdx/types.js').MDXModule} MDXModule
* @typedef {import('vue').Component} AnyComponent
*/
Expand All @@ -9,13 +7,9 @@ import assert from 'node:assert/strict'
import test from 'node:test'
import {compile, run} from '@mdx-js/mdx'
import {MDXProvider, useMDXComponents} from '@mdx-js/vue'
import * as runtime_ from 'vue/jsx-runtime'
import * as runtime from 'vue/jsx-runtime'
import * as vue from 'vue'

const runtime = /** @type {{Fragment: Fragment, jsx: Jsx, jsxs: Jsx}} */ (
/** @type {unknown} */ (runtime_)
)

// Note: a regular import would be nice but that completely messes up the JSX types.
const name = '@vue/server-renderer'
/** @type {{default: {renderToString(node: unknown): string}}} */
Expand Down