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.

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

Vite: Use mdx2 babel pre-processing #20241

Merged
merged 1 commit into from
Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions code/lib/builder-vite/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,11 @@
"@storybook/client-logger": "7.0.0-beta.6",
"@storybook/core-common": "7.0.0-beta.6",
"@storybook/csf-plugin": "7.0.0-beta.6",
"@storybook/mdx2-csf": "next",
"@storybook/mdx2-csf": "0.0.4-canary.30.c978d7f.0",
"@storybook/node-logger": "7.0.0-beta.6",
"@storybook/preview": "7.0.0-beta.6",
"@storybook/preview-api": "7.0.0-beta.6",
"@storybook/types": "7.0.0-beta.6",
"@vitejs/plugin-react": "^2.0.0",
"browser-assert": "^1.2.1",
"es-module-lexer": "^0.9.3",
"express": "^4.17.3",
Expand Down
39 changes: 3 additions & 36 deletions code/lib/builder-vite/src/plugins/mdx-plugin.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,9 @@
import type { Options } from '@storybook/types';
import type { Plugin } from 'vite';
import { createFilter } from 'vite';
import reactVite from '@vitejs/plugin-react';

const isStorybookMdx = (id: string) => id.endsWith('stories.mdx') || id.endsWith('story.mdx');

function injectRenderer(code: string) {
return `
import React from 'react';
${code}
`;
}

// HACK: find a better way to do this, ideally avoiding @vitejs/plugin-react entirely.
// We're just using it to run the mdx with jsx through babel
// @ts-expect-error We're forcing the plugin shape here
const viteBabel: Plugin | undefined = reactVite({ fastRefresh: false }).find(
// @ts-expect-error we know these have names, and what the shape will be
(p) => p.name === 'vite:react-babel'
);

/**
* Storybook uses two different loaders when dealing with MDX:
*
Expand All @@ -35,7 +19,7 @@ export function mdxPlugin(options: Options): Plugin {
return {
name: 'storybook:mdx-plugin',
enforce: 'pre',
async transform(src, id, transformOptions) {
async transform(src, id) {
if (!filter(id)) return undefined;

const { compile } = await import('@storybook/mdx2-csf');
Expand All @@ -46,33 +30,16 @@ export function mdxPlugin(options: Options): Plugin {
},
});

const mdxCode = String(
const code = String(
await compile(src, {
skipCsf: !isStorybookMdx(id),
...mdxLoaderOptions,
Copy link
Member

Choose a reason for hiding this comment

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

You're missing jsxOptions

Copy link
Member Author

Choose a reason for hiding this comment

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

Where should they come from? Is this something we want the user to be able to set? If so, why?

Copy link
Member

@ndelangen ndelangen Dec 14, 2022

Choose a reason for hiding this comment

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

Yeah, I recon we want to get them from a similar place as mdxCompilerOptions: from a preset value.

Would you like me to pick that up? I can likely get that done by end-of-day.

})
);

const modifiedCode = injectRenderer(mdxCode);

// Hooks in recent rollup versions can be functions or objects, and though react hasn't changed, the typescript defs have
const rTransform = viteBabel?.transform;
const transform = rTransform && 'handler' in rTransform ? rTransform.handler : rTransform;

// It's safe to disable this, because we know it'll be there, since we added it ourselves.
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const result = await transform!.call(this, modifiedCode, `${id}.jsx`, transformOptions);

if (!result) return modifiedCode;

if (typeof result === 'string') return result;

const { code, map: resultMap } = result;

return {
code,
map:
!resultMap || typeof resultMap === 'string' ? resultMap : { ...resultMap, sources: [id] },
map: null, // TODO: update mdx2-csf to return the map
};
},
};
Expand Down
12 changes: 10 additions & 2 deletions code/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6021,14 +6021,13 @@ __metadata:
"@storybook/client-logger": 7.0.0-beta.6
"@storybook/core-common": 7.0.0-beta.6
"@storybook/csf-plugin": 7.0.0-beta.6
"@storybook/mdx2-csf": next
"@storybook/mdx2-csf": 0.0.4-canary.30.c978d7f.0
"@storybook/node-logger": 7.0.0-beta.6
"@storybook/preview": 7.0.0-beta.6
"@storybook/preview-api": 7.0.0-beta.6
"@storybook/types": 7.0.0-beta.6
"@types/express": ^4.17.13
"@types/node": ^16.0.0
"@vitejs/plugin-react": ^2.0.0
browser-assert: ^1.2.1
es-module-lexer: ^0.9.3
express: ^4.17.3
Expand Down Expand Up @@ -6779,6 +6778,15 @@ __metadata:
languageName: unknown
linkType: soft

"@storybook/mdx2-csf@npm:0.0.4-canary.30.c978d7f.0":
version: 0.0.4-canary.30.c978d7f.0
resolution: "@storybook/mdx2-csf@npm:0.0.4-canary.30.c978d7f.0"
dependencies:
loader-utils: ^2.0.4
checksum: a9e8c36673f08ec294566535f61c20440ede8ccf834006506efd183ed0dc566dbb6863776088b628b8cd3f96662ed79c8c427de4617851eae5d60b3a0ae1b23f
languageName: node
linkType: hard

"@storybook/mdx2-csf@npm:next":
version: 0.1.0-next.7
resolution: "@storybook/mdx2-csf@npm:0.1.0-next.7"
Expand Down