Fix image snapshots setup in official-storybook #8932
Conversation
Hypnosphi
requested review from
alterx,
igor-dv,
ndelangen,
thomasbertet,
tmeasday and
usulpro
as code owners
|
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/storybook/monorepo/iify2jvoe |
| output.stories = stories.map( | ||
| (pattern: string | { path: string; recursive: boolean; match: string }) => { | ||
| const { path: basePath, recursive, match } = toRequireContext(pattern); | ||
| return require.context( |
There was a problem hiding this comment.
🤔 how does this work in Jest? I didn't think require.context() existed?
There was a problem hiding this comment.
In our own setup, it's Option 1: Plugin (babel-plugin-require-context-hook)
There was a problem hiding this comment.
Oh, right but this isn't universal. For instance:
- Users may be using the babel macro
- This code was added to actually avoid needing to use babel at all if they are using the
main.jsentrypoint.
What was the issue with what was there before w/ more typical require()s?
There was a problem hiding this comment.
Also it is unlikely that user's code is going to run their babel plugins over files in node_modules. Or is all this being compiled when we compile addon/storyshots?
There was a problem hiding this comment.
Or is all this being compiled when we compile addon/storyshots?
It is but require-context plugin isn't applied right now, I'll add that, thanks for noting.
UPD: it's tsc not babel, so I think I'll just import babel-plugin-require-context-hook/register and use global.__requireContext directly
What was the issue with what was there before w/ more typical require()s?
We used different libraries for turning globs into files in core and storyshots which sometimes led to differences. For instance, "regex or" patterns (.stories.(js|tsx|mdx)) that we use in main.js aren't supported by node-glob
# Conflicts: # addons/storyshots/storyshots-core/package.json
| if (getCustomBrowser) { | ||
| browser = await getCustomBrowser(); | ||
| } else { | ||
| // eslint-disable-next-line global-require | ||
| const puppeteer = require('puppeteer'); |
There was a problem hiding this comment.
You may want to try TS dynamic import: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-4.html#dynamic-import-expressions & https://basarat.gitbooks.io/typescript/docs/project/dynamic-import-expressions.html
I don't know if it will work on this case because I don't have a full knowledge of how storyshots-puppeteer works.
There was a problem hiding this comment.
Does it work in Node 10?
Issue:
yarn image-snapshotsinexamples/official-storybookdoesn't workWhat I did
toRequireContextin storyshots to match the behaviour of@storybook/coreglob-regexwithmicromatch.makeReas the former only supports very basic patternsstoryshot-puppeteersetup and tests