Add JSX transpilation to compiler-generated code #30

ndelangen · 2022-12-13T15:45:53Z

default jsxRuntime = 'automatic'

Issue: #

What Changed

This adds a new third argument to the compileAsync function: JsxOptions which mimicks the options you can pass to the babel/preset-react. Unfortunately that interface isn't public/exposed, so I copied it from their docs: https://babeljs.io/docs/en/babel-preset-react#with-a-configuration-file-recommended

I made the interface for compileSync the same as compileAsync.

How to test

The unit test say: yes
We need to take this canary release and test it in the storybook repo / sandboxes.

Change Type

Version

Published prerelease version: v1.0.0-next.0

Changelog

💥 Breaking Change

Add JSX transpilation to compiler-generated code #30 (@ndelangen)

🚀 Enhancement

Remove uses of AddContext #14 (@tmeasday)

🐛 Bug Fix

Export CompileOptions #28 (@joshwooding)
Add MDX compiler option overrides #27 (@shilman)
Bump version of loader-utils #26 (@eirslett-visma)
Fix canvas mdxSource generation and add tests #25 (@zhyd1997 @shilman)
Upgrade example to SB7 #24 (@shilman)
Prebundle with tsup #23 (@shilman)
Add mdx tag for SB7 support #22 (@shilman)
Remove JSX from compiler's page output #20 (@IanVS @shilman)
chore: Moved @babel/types to dependencies #19 (@bryanjtc)

⚠️ Pushed to `next`

Fix yarn.lock (@shilman)

Authors: 8

Bryan Thomas (@bryanjtc)
Eirik Sletteberg (@eirslett-visma)
Ian VanSchooten (@IanVS)
Josh Wooding (@joshwooding)
Michael Shilman (@shilman)
Norbert de Langen (@ndelangen)
Tom Coleman (@tmeasday)
Yadong Zhang (@zhyd1997)

…e JSX runtime. default jsxRuntime = 'automatic'

ndelangen · 2022-12-13T15:49:01Z

I was blocked from committing to the repo, by a pre-commit hook, that I could not get to work on my machine.
I also do not have prettier installed in vs-code, and don't want to do so, due to that would conflict with all my other repos do using eslint.

So I ran yarn prettier . --write every time I wanted code formatted, but that formatted a ton more stuff. Maybe I shouldn't have, but seems to to me that is the intended use?

ndelangen · 2022-12-13T16:11:04Z

If this is to replace the mdx/jsx/react plugin for vite ( or rather: make it no longer required ), it might need more work to make HMR work..? WDYT @IanVS

IanVS

I've opened storybookjs/storybook#20241 which pulls in the canary for this change, and it works a treat!

IanVS · 2022-12-13T16:07:10Z

src/index.ts


-export { mdxSync as compileSync };
+  if (skipCsf) {
+    const mdxResult = await mdxCompile(input, options);


Nit: it looks like this could be moved above the if-check.

IanVS · 2022-12-13T16:09:28Z

src/jsx.ts

+
+export const transformJSXAsync = async (input: string, jsxOptions: JSXOptions) => {
+  const babelOptions = getBabelOptions(jsxOptions);
+  const { code } = await transformAsync(input, babelOptions);


In the future we could explore transforming jsx with esbuild, which might be faster. But that feels like a separate effort.

IanVS · 2022-12-14T00:09:57Z

src/sb-mdx-plugin.ts

@@ -1,4 +1,3 @@
-import { parse } from '@babel/parser';


It looks like this can be removed from package.json now, it's not used elsewhere.

IanVS · 2022-12-14T00:16:55Z

src/index.ts

+    return mdxResult.toString();
+  }
+
+  const mdxResult = mdxCompileSync(input, options);


Nit: This could also move up above the if (skipCsf) check.

IanVS · 2022-12-14T00:24:23Z

src/jsx.ts

+  const babelOptions = getBabelOptions(jsxOptions);
+  const { code } = transformSync(input, babelOptions);
+
+  return code;


It would be ideal if we could get a hold of the map as well, to enable sourcemaps.

Thank for the review @IanVS, super appreciated. I'll leave the code as is for now, if the code works as is, I'd rather merge and release.

I do believe sourcemaps would be really great to have, but I feel like if I were tasked, I'd likely rewrite the entire thing, yet i don't understand 3/4th of the code.

Fair enough. transformSync returns a map, so we'd need to return {code, map} from this function, and pass it along as the return from the compile call. Then it's a matter of updating the code that is using this package. But it looks like maybe it's only used in two spots. I'll give it a shot and see what happens. Now seems like the time to do it, while we're making breaking changes.

shilman · 2022-12-14T00:38:58Z

src/index.ts

@@ -148,37 +150,72 @@ export const postprocess = (code: string, extractedExports: string) => {
  const first = lines.shift();

  return [
-    first,


what's going on here?

I'm dropping that first line, because it's a comment instructing the JSX compiler to use the options passed to it from mdxCompilerOptions.

We want those options to be disregarded, because we want to use the options from jsxOptions instead.

I didn't find a way to have the mdxCompiler NOT generate the comment. Though I gave up after like 2 attempts.
The code to extract the first line was already there. I didn't understand why it was extracted and then added back anyway, it made no sense to me. But it was close enough to what I wanted to do: remove the first line comment generated by the mdxCompiler.

Must've been part of some legacy behavior 🤷

Thanks for the clarification! SGTM.

refactor to transpile JSX at the end of the process, ensuring a singl…

c978d7f

…e JSX runtime. default jsxRuntime = 'automatic'

ndelangen self-assigned this Dec 13, 2022

ndelangen added enhancement New feature or request major Increment the major version when merged labels Dec 13, 2022

ndelangen requested a review from shilman December 13, 2022 15:46

ndelangen requested a review from IanVS December 13, 2022 15:59

shilman changed the title ~~refactor to transpile JSX at the end of the process, ensuring a single JSX runtime.~~ Refactor to transpile JSX at the end of the process, ensuring a single JSX runtime Dec 14, 2022

IanVS mentioned this pull request Dec 14, 2022

Vite: Use mdx2 babel pre-processing storybookjs/storybook#20241

Merged

IanVS approved these changes Dec 14, 2022

View reviewed changes

shilman reviewed Dec 14, 2022

View reviewed changes

shilman changed the title ~~Refactor to transpile JSX at the end of the process, ensuring a single JSX runtime~~ Add JSX transpilation to compiler-generated code Dec 14, 2022

shilman merged commit 30274fb into next Dec 14, 2022

shilman deleted the norbert/sb-1086-sb20190-bug-vite4-support-missing-v2 branch December 14, 2022 11:27

github-actions bot mentioned this pull request Dec 14, 2022

JSX: Ignore the user's babel config #31

Merged

5 tasks

IanVS mentioned this pull request Dec 14, 2022

Only support webpack 5, expose some types #32

Merged

5 tasks

github-actions bot mentioned this pull request Dec 30, 2022

Replace 'mdx' tag with 'stories-mdx' #33

Merged

5 tasks

github-actions bot mentioned this pull request Jan 6, 2023

Fix type exports #34

Merged

5 tasks

github-actions bot mentioned this pull request Jan 21, 2023

Add debug output to loader.js #17

Merged

5 tasks

IanVS mentioned this pull request Jan 22, 2023

Fully support Vite 4, remove @vitejs/plugin-react, update @storybook/mdx1-csf storybookjs/builder-vite#548

Merged

github-actions bot mentioned this pull request Mar 16, 2023

Throw descriptive error when 'of' prop is detected #37

Merged

5 tasks

This was referenced Mar 31, 2023

Upgrade to SB7.0 #38

Merged

Release 1.0.0 #39

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add JSX transpilation to compiler-generated code #30

Add JSX transpilation to compiler-generated code #30

ndelangen commented Dec 13, 2022 •

edited by github-actions bot

Loading

ndelangen commented Dec 13, 2022

ndelangen commented Dec 13, 2022 •

edited

Loading

IanVS left a comment

IanVS Dec 13, 2022

IanVS Dec 13, 2022

IanVS Dec 14, 2022

IanVS Dec 14, 2022

IanVS Dec 14, 2022

ndelangen Dec 14, 2022

IanVS Dec 14, 2022

shilman Dec 14, 2022

ndelangen Dec 14, 2022

shilman Dec 14, 2022

Add JSX transpilation to compiler-generated code #30

Add JSX transpilation to compiler-generated code #30

Conversation

ndelangen commented Dec 13, 2022 • edited by github-actions bot Loading

What Changed

How to test

Change Type

Version

💥 Breaking Change

🚀 Enhancement

🐛 Bug Fix

⚠️ Pushed to next

Authors: 8

ndelangen commented Dec 13, 2022

ndelangen commented Dec 13, 2022 • edited Loading

IanVS 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

ndelangen commented Dec 13, 2022 •

edited by github-actions bot

Loading

⚠️ Pushed to `next`

ndelangen commented Dec 13, 2022 •

edited

Loading