-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Storyshots fails with MDX stories via docs addon #7223
Comments
This is also blocking this PR #7222 so we'll need to deal with it sooner rather than later. |
@danielknell For now, I just added the There's already a webpack loader that transforms the stories into JSX. Do you have any interest in trying to get that working for jest? I probably don't have time to look at this much more for awhile, but I'd be happy to answer questions if you want to tackle it. The webpack setup is here: https://github.com/storybookjs/storybook/blob/next/addons/docs/common/preset.js |
Out of curiosity: Will Storyshots still work with MDX files? E.g. for
so that both stories get picked up? |
@rwieruch See my comment
|
const createCompiler = require("@storybook/addon-docs/mdx-compiler-plugin");
const mdx = require("@mdx-js/mdx");
const babel = require("babel-jest");
const deasyncPromise = require("deasync-promise");
const compilers = [createCompiler({})];
module.exports = {
process(src, filename, config, options) {
let result = deasyncPromise(
mdx(src, { compilers: compilers, filepath: filename }),
);
result = `/* @jsx mdx */
import React from 'react'
import { mdx } from '@mdx-js/react'
${result}
`;
return babel.process(result, filename, config, options);
},
}; hacky as fuck but seems to work...? using the following in jest config {
"transform": {
"^.+\\.[tj]sx?$": "babel-jest",
"^.+\\.mdx?$": "<rootDir>mdx.loader.js",
}
} |
Nice @danielknell. Would somebody mind putting that into a PR as |
@shilman it assumes you are using babel-jest, is that a safe assumption? thats also probably not the right way to chain jest transforms either but i couldn't see any other ways in the docs... |
@danielknell I think we could require users to use babel-jest if that's what's needed to make it work? Obviously the fewer requirements the better tho. |
i looked into it more and it seems that is how other people chain transforms, and there was some unneeded stuff in there, so i've updated the comment and wrapped it up in a pr |
Just a note re: the above code snippet if you're using it while waiting for #7330 to drop, you don't need to use See: #7330 (comment) |
Ta-da!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.0-alpha.27 containing PR #8189 that references this issue. Upgrade today to try it out! You can find this prerelease on the Closing this issue. Please re-open if you think there's still more to do. |
Finished except for instrumenting |
Hi, I have just installed Storybook 5.3.14, addon-docs and addon-storyshots and tried to run Storyshot.test.js, all of it as given in the instructions for this version, but it fails with
I have only one story in src/Home import { Meta, Story, Preview } from "@storybook/addon-docs/blocks";
import { Home } from "./Home";
<Meta title="Home" component={Home} />
# Home
The Home
<Preview>
<Story name="Home">
<Home />
</Story>
</Preview> Just for the record, .js format works import React from "react";
import { Home } from "./Home";
export default {
component: Home,
title: "Home"
};
export const base = () => <Home />; I debugged it a little bit and found that |
I faced a similar problem in CRA.
@@ -29,6 +29,6 @@
import { mdx } from '@mdx-js/react'
${mdx.sync(src, { compilers, filepath: filename })}
`;
- return getNextTransformer(filename, config).transformSource(filename, result, instrument);
+ return getNextTransformer(filename, config).transformSource(filename + '.jsx', result, instrument);
},
}; ☝️ I don't know if this is a bug. It might be better to create custom transformer.
@@ -51,17 +51,17 @@
var _memoizerific = _interopRequireDefault(require("memoizerific"));
-var _jsx = _interopRequireDefault(require("react-syntax-highlighter/dist/esm/languages/prism/jsx"));
+var _jsx = _interopRequireDefault(require("react-syntax-highlighter/dist/cjs/languages/prism/jsx"));
-var _bash = _interopRequireDefault(require("react-syntax-highlighter/dist/esm/languages/prism/bash"));
+var _bash = _interopRequireDefault(require("react-syntax-highlighter/dist/cjs/languages/prism/bash"));
-var _css = _interopRequireDefault(require("react-syntax-highlighter/dist/esm/languages/prism/css"));
+var _css = _interopRequireDefault(require("react-syntax-highlighter/dist/cjs/languages/prism/css"));
-var _markup = _interopRequireDefault(require("react-syntax-highlighter/dist/esm/languages/prism/markup"));
+var _markup = _interopRequireDefault(require("react-syntax-highlighter/dist/cjs/languages/prism/markup"));
-var _tsx = _interopRequireDefault(require("react-syntax-highlighter/dist/esm/languages/prism/tsx"));
+var _tsx = _interopRequireDefault(require("react-syntax-highlighter/dist/cjs/languages/prism/tsx"));
-var _typescript = _interopRequireDefault(require("react-syntax-highlighter/dist/esm/languages/prism/typescript"));
+var _typescript = _interopRequireDefault(require("react-syntax-highlighter/dist/cjs/languages/prism/typescript"));
var _reactSyntaxHighlighter = require("react-syntax-highlighter"); fixed in v6.0.0-alpha.7 (#9780) |
@Yama-Tomo Do you mean the |
@CjChoiNZ Yes. (no problem anywhere before |
Same error. "storyshots found 0 stories" :-( |
@Yama-Tomo
And confirmed use this path I defined the
And the result of transform still displays
Did I miss something? |
@CjChoiNZ You need to customize the order of
module.exports = {
...
jest: function (config) {
...
config.transform = {
'^.+\\.mdx$': '/path/to/transformer(e.g. @storybook/addon-docs/jest-transform-mdx)',
...config.transform
}
return config
}
} |
@Yama-Tomo |
I've tried the
I've got a
My
My
My
Non-MDX stories are picked up and test just fine. |
For me also no MDX stories are not picked up but I'm using |
Faced the same issue recently. Solved by the following trick it
|
Just faced this same issue with Storybook v6:
|
We've encountered the same issue, but it had nothing to do with storybook. In our case the files weren't getting recognised by jest. Adding this line to jest storyshots config fixed the problem:
https://jestjs.io/docs/en/configuration#modulefileextensions-arraystring |
As pointed by @Yama-Tomo the order of the module.exports = {
overrideJestConfig: ({ jestConfig }) => {
jestConfig.transform = {
".+\.mdx": `${resolveApp()}/node_modules/@storybook/addon-docs/jest-transform-mdx`,
...jestConfig.transform,
}
return jestConfig;
}
}; |
@shilman is this fixed now ? jest.config.js
|
A solution for me was just to simply remove the mock below from our JestSetup file, and just move it in the test which was failing because of storybook.
|
Describe the bug
If you try and use storyshots while using the docs addon, jest will try and import the MDX file, which will result in syntax errors (it not understanding markdown code and all)
To Reproduce
Steps to reproduce the behavior:
Expected behavior
Jest should create snapshots based on stories in MDX file like it does for jsx/tsx files or ignore them.
System:
Additional context
Not really that important as it's in alpha and i'm just poking at what the future might look like, but will probably cause lots of confusion after a stable release, the docs addon is awesome btw, looking forward to the final version.
I am thinking a webpack loader will be needed to transform the MDX a regular JSX file defining the stories?
The text was updated successfully, but these errors were encountered: