Support jsx dev runtime

[remcohaszing]

The JSX dev runtime exposes more debugging information to users. For example, React exposes positional information through React Devtools.

Although #2035 started off as an issue to support the __source prop, I have refrained from actually supporting the __source prop, as it’s specific to React. The automatic dev runtime supports this information without patching props.

Closes #2035

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
mdx	✅ Ready (Inspect)	Visit Preview	Dec 8, 2022 at 3:38PM (UTC)

[remcohaszing]

The test above uses an undocumented private property of React elements. The test below asserts the generated source code. None of these tests is ideal, but I’m not sure which is worse. I suggest to keep only one of them though.

[codecov-commenter]

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (6085e65) compared to base (38c2d46).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff            @@
##              main     #2045   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        22           
  Lines         2050      2060   +10     
=========================================
+ Hits          2050      2060   +10     

Impacted Files	Coverage Δ
packages/mdx/lib/core.js	100.00% <100.00%> (ø)
packages/mdx/lib/plugin/recma-jsx-build.js	100.00% <100.00%> (ø)
packages/mdx/lib/util/resolve-evaluate-options.js	100.00% <100.00%> (ø)
packages/mdx/lib/plugin/recma-stringify.js	100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[wooorm]

Looks good, one idea.

I wonder whether we should document that automatic runtimes need the /jsx-dev-runtime for this to work. E.g., here: https://mdxjs.com/packages/mdx/#optionsjsxruntime

I believe we have some types for jsx, jsxs somewhere, should they include jsxDev?

Should jsxDev be needed in evaluate, run? https://mdxjs.com/packages/mdx/#evaluatefile-options, https://mdxjs.com/packages/mdx/#options-1

We should probably include some docs in https://mdxjs.com/packages/mdx/#optionsdevelopment as well

[wooorm]

Maybe good to move this one next to the jsx tests (L826), in a new test for automatic runtime output?
Sometimes the output changes a bit and then its easier if all the output tests are next to eachother?

[remcohaszing]

Looks good, one idea.

I wonder whether we should document that automatic runtimes need the /jsx-dev-runtime for this to work. E.g., here: https://mdxjs.com/packages/mdx/#optionsjsxruntime

I believe we have some types for jsx, jsxs somewhere, should they include jsxDev?

Should jsxDev be needed in evaluate, run? https://mdxjs.com/packages/mdx/#evaluatefile-options, https://mdxjs.com/packages/mdx/#options-1

We should probably include some docs in https://mdxjs.com/packages/mdx/#optionsdevelopment as well

Good points. I’ll update the types and docs.

I think maybe this should be a separate option. This change will be fine for React / Preact users, but other frameworks might not support the JSX dev runtime. I.e. hastscript and xastscript don’t support it. We may add it in these two specific packages, because they are part of unified, but there may be other JSX runtimes out there that don’t support it either.

Of course people can set development: false, but if users have to do so, this would be a breaking change, and they lose other benefits of MDX development mode.

[wooorm]

It might indeed break folks. I wonder if it can be considered a bug on the part of those runtimes, which can be solved by users by setting development: false.

Alternatively, could we add a runtime check: as we’re in development, we could import both development and production, defaulting to production if development is missing, potentially with a warning, and making the switch in v3?

[remcohaszing]

Should jsxDev be needed in evaluate, run? https://mdxjs.com/packages/mdx/#evaluatefile-options, https://mdxjs.com/packages/mdx/#options-1

Yes. The automatic dev runtime needs jsxDEV, whereas the automatic production runtime needs jsx and jsxs. I suggest that for evaluate and run, we don’t accept the development option. Instead, we set development as Boolean(jsxDEV). This means that is the user passes a dev runtime instead of a production runtime, MDX will automatically switch to dev mode for compilation.

[wooorm]

WDYT about:

could we add a runtime check: as we’re in development, we could import both development and production, defaulting to production if development is missing, potentially with a warning, and making the switch in v3?

I feel a lot for this, to prevent a breaking change. And as it is dev, it’s fine adding some more code.

[remcohaszing]

It might indeed break folks. I wonder if it can be considered a bug on the part of those runtimes, which can be solved by users by setting development: false.

I don’t consider this to be a bug. A JSX automatic runtime implementation can fully function without supporting the dev runtime in Babel, TypeScript, SWC, and ESBuild. Although I wouldn’t mind making this assumption in a breaking change for MDX 3.0. Afaik most major JSX implementations support this.

Alternatively, could we add a runtime check: as we’re in development, we could import both development and production, defaulting to production if development is missing, potentially with a warning, and making the switch in v3?

I don’t understand. As far as I understand you mean to transform this (simplified):

# Hello

into this:

import { jsx } from 'framework/jsx-runtime'
import { jsxDEV } from 'framework/jsx-dev-runtime'

export default function MDXContent() {
  return jsx('h1', { children: 'Hello' })
}

This still breaks if the framework doesn’t support the jsx-dev-runtime.

I suggest a build time warning (once) instead if development !== devRuntime (or however we name the new option).

Alternatively we could keep a hardcoded list of frameworks that support the automatic dev runtime. and just apply it for those. This isn’t elegant, but it would make it a non-breaking change without bothering any users.

Do you know if it’s possible to query the npm registry to find out if such JSX runtimes even exist at all? If hastscript and xastscript are the only ones, we could just update this and not bother about this compatibility issue at all.

[wooorm]

in Babel, TypeScript, SWC, and ESBuild.

Are you sure? I was under the impression that, if development is turned on in Babel, it takes jsxDEV, and thus crashes if that isn‘t there?

[wooorm]

Hmm, taking TS as an example, instructed to run in the automatic dev runtime:

<a />

->

import { jsxDEV as _jsxDEV } from "react/jsx-dev-runtime";
const _jsxFileName = "file:///input.tsx";
_jsxDEV("a", {}, void 0, false, { fileName: _jsxFileName, lineNumber: 1, columnNumber: 1 }, this);

I was missing the dev- part in the import URL in my mind. Without that change, we could generate something like:

import * as runtime from "react/jsx-runtime";
let _jsx = runtime.jsx
if (runtime.jsxDEV) {
  _jsx = runtime.jsxDEV
} else {
  console.log('stuff')
}

...

But as it’s a different specifier, we can’t fix it at runtime...

[remcohaszing]

in Babel, TypeScript, SWC, and ESBuild.

Are you sure? I was under the impression that, if development is turned on in Babel, it takes jsxDEV, and thus crashes if that isn‘t there?

My bad, you are right about Babel. I know I’m right about TypeScript and ESBuild. I’m unsure about SWC.

I believe the Babel transform defaults to looking at Babel’s development mode, but it has an undocumented plugin specific development override, so the dev transform could be disabled explicitly in development mode.

[wooorm]

But as /jsx-dev-runtime is used by bundler/builder/etc, then they all throw for such runtimes in development already? In which case, it’s fine for us to follow that?

[remcohaszing]

No, TypeScript uses the jsx compiler option, which can be set to either react (classic runtime), react-jsx (automatic runtime), or react-jsxdev (automatic dev runtime). ESBuild follows this. TypeScript doesn’t really have the development context that Babel does.

[remcohaszing]

Anyway, I suspect the number of impacted users would be very low. React, Preact, and Emotion support the automatic runtime, Vue doesn’t use JSX transforms, we control hastscript and xastscript.

[wooorm]

Two things. TS still takes /jsx-dev-runtime.

That's what I'm talking about.

Every tool takes that export specifier. So we can use it too.
If it is missing in a runtime, it'll throw an error. But every tool throws an error. So it's fine.

And, we need to take jsxDEV from it. It might not exist. From what I understand, several runtimes don't have it.
We can take it, and jsx, and default to the latter with a warning

There is a large chance that people link the same file, with the same exports, to both export entries. Which is fine, whether they export jsxDEV from it or not, in that case.

There is then only a tiny chance that someone has an export for /jsx-dev-runtime, without a jsxDEV, which is clearly wrong, and can crash.

[manucorporat]

any update on this?

[wooorm]

The comment before your comment is the last update on this!

I think that, to land this without breaking, we need to import * as xxx from 'xxx/jsx-dev-runtime', and fall back to the regular xxx.jsx/xxx.jsxs functions, with a warning, when jsxDEV does not exist in xxx.
If someone is able to test particularly with svelte/solid that this isn’t needed, then we can do without.

I am fine with introducing a crash when /jsx-dev-runtime is not in the export map of the runtime, because Babel, esbuild, typescript, etc, also would fail on that package.

[manucorporat]

So at Qwik we have MDX support in our meta framework and recently introduced a click-to-component feature that leverages this:
https://www.loom.com/share/e1c6261f10814362a76daeb7ee3f120c

we tried using the PR and works great!

I understand the worry about breaking changes, but virtually all the tools supports JSX-automatic, emit what MDX is doing in this PR right now (esbuild, rollup tsx plugins, and swc). It's part of JSX-automatic, if it breaks, i think the framework authors should fix it should fix it by exporting jsxDEV.

In the meantime, we are forking MDX and ship this feature :) but we would love not having to do it.

Thanks for your early response! this is such a cool project!

[wooorm]

if it breaks, i think the framework authors should fix it should fix it by exporting jsxDEV.

I don’t want all Solid or Svelte users to suddenly break in a patch.

we are forking MDX and ship this feature :) but we would love not having to do it.

Perhaps you can spend the time to verify my above question. As that time is the thing that’s missing.

[remcohaszing]

I intend to finish this, but currently other issues have higher priority for me. Feel free to work on it if this has high priority for you.

PS: It’s really cool to see how this is leveraged in Qwik!

[manucorporat]

I tried to look into following the implementation suggested by @wooorm because i would love this PR being unblocked!
The generation of the proposed changed in outside the scope of MDX, but it would be a change in estree-util-build-jsx.

I dont think it's realistic and good idea to make MDX have a different transform to jsxDev, when everything is doing the same.

Instead, if what we want is to prevent a breaking change, i would create a new option, like jsxDevelopment: boolean, so effectively, jsxDev != development, then in the next major, we can merge jsxDev and development

[manucorporat]

Solid:
https://github.com/solidjs/solid/blob/main/packages/solid/h/jsx-runtime/src/index.ts#L10
https://github.com/high1/solid-jsx/blob/main/package.json#L9-L10
https://github.com/high1/solid-jsx/blob/main/src/jsx-runtime.ts#L77

Preact/React also implements

Svelte does not use JSX

Vue is not using jsx-automatic

[wooorm]

Thanks for looking that up!
I am particularly worried about what we recommend on our site. As that’s what people’ve been using for a year: https://mdxjs.com/docs/getting-started/#jsx.
The are solid and svelte JSX implementation mentioned there, each support a /jsx-dev-runtime entry in the export map (you found the solid alternative there, too).

However, you also mention that solid itself now support the JSX runtime. That’s new. I remember them initially being against having that. And it seems they do expose jsxDEV, but they do not have a /jsx-dev-runtime entry!

@ryansolid I think Solid needs a /jsx-dev-runtime entry, which can be the same as this. That’s what the automatic JSX runtime will use, when configured through esbuild/typescript/babel/mdx, when development is turned on.

[manucorporat]

@wooorm For me understanding, the mdx integration with solid does not use the same jsx-runtime as the one in solid, cuz they have a total custom JSX transform, none of mdx is directly useful, instead MDX relies on solid-jsx. Notice that solid itself does not have vdom. solid-jsx is the external package that bring the compatibility

https://github.com/high1/solid-jsx/blob/main/package.json#L9-L10
https://github.com/high1/solid-jsx/blob/main/src/jsx-runtime.ts#L77

which does support jsxDEV (basically same as jsx)

[manucorporat]

Preact, solid-jsx, svelte-jsx, vue, qwik... so far all frameworks of libraries support the jsx mode, follow the defacto spec of having jsx-runtime and jsx-dev-runtime, the second exporting a jsxDev.

Any tool supporting jsx transform in development mode will emit jsxDev already, importing jsx-dev-runtime. Imo, it's perfectly safe to merge this PR, since:

• all mentioned frameworks support it
• it's outside the scope of mdx this transform(it's inestree-util-build-jsx, and it's safe for them)
• the rest of tools already already doing this if you enable JSX-automatic

The amazing side effect of this PR is enable amazing dev tools already existing in this frameworks to jump directly to the mdx/md file!! and make their devtools better integrate.

[wooorm]

the mdx integration with solid does not use the same jsx-runtime as the one in solid, cuz they have a total custom JSX transform, none of mdx is directly useful, instead MDX relies on solid-jsx. Notice that solid itself does not have vdom. solid-jsx is the external package that bring the compatibility

This is historically correct. Currently, Solid does provide its own automatic JSX runtime. It seems to do so for a while already. You mentioned this above yourself:

• https://github.com/solidjs/solid/blob/c993eb0c6fb7e39772759f1f7605fdb27dbf5863/packages/solid/h/jsx-runtime/src/index.ts#L10
• https://github.com/solidjs/solid/tree/c993eb0c6fb7e39772759f1f7605fdb27dbf5863/packages/solid/h

solid-jsx is the external package that bring the compatibility

It used to. And we link to it. As you show though, Solid now provides an automatic JSX runtime too.

which does support jsxDEV (basically same as jsx)

There are two parts that are needed for an automatic runtime, development or not, to work:

• Export the correct variables (i.e., jsx and friends, or jsxDEV)
• Export the correct specifier (i.e., ./jsx-runtime or ./jsx-dev-runtime)

As you and I checked and verified above, all projects (solid-jsx, svelte-jsx, react, preact, emotion) do both. But solid itself does not export ./jsx-dev-runtime. That is a problem.

Imo, it's perfectly safe to merge this PR

It seems that solid itself supports only part of the needed API. I believe my previous comment shows how Solid can solve that: #2045 (comment).

The amazing side effect of this PR [...]

That is the purpose of this PR. The side effect of this PR would be breaking several Solid users in a patch release.

[manucorporat]

Alright! https://github.com/solidjs/solid/pull/1393/files

could we unblock this PR once solid merge the fix? :)

[wooorm]

Yes, that unblocks the PR, thank you! (Note though that I don’t know Solid enough to review the PR)

[manucorporat]

yep! no worries, reached Ryan to see what we can do about it

[manucorporat]

Solid merged the PR to add jsx-dev-runtime, just waiting for a release from @ryansolid

solidjs/solid#1393

[remcohaszing]

The only thing missing then is support for evaluate / run and documentation updates.

I see two solution to determine whether or not to use development mode:

1. Throw an error if runtime properties are missing:

   if (options.development) {
     if (!options.jsxDEV) {
       throw new Error('Missing option jsxDEV required in development mode')
     }
   } else {
     if (!options.jsx) {
       throw new Error('Missing option jsx required in production mode')
     }
   
     if (!options.jsxs) {
       throw new Error('Missing option jsxs required in production mode')
     }
   }

2. Try to recover. Determine development mode based on the runtime passed.

[wooorm]

1) makes the most sense to me.
There's also 3): only use jsxDEV if both development and jsxDEV are given. In this case I think it's a clear programmer error I think, so I'm fine throwing, so approach 1

[remcohaszing]

Ok, I’ll finish this PR then.

[manucorporat]

so exciting! thank you team! you guys rock! @remcohaszing @wooorm

[manucorporat]

Just confirming, solidjs merged the PR to support jsx-dev-runtime and made a release with the fix!

[wooorm]

Released, thanks both! :)

[kherock]

This needs to increment the minimum version of estree-util-build-jsx to ^2.1.0. I was getting the error

TypeError: _jsx is not a function

until I was able to track down this dependency in my lockfile!

[remcohaszing]

The version range ^2.1.0 is a subset of^2.0.0. So no version bump is needed.

The policy in the unified / mdx ecosystem is to use version ranges of the form ^x.0.0.