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

React: Support JSX react transform introduced in 16.14.0 #12899

Merged
merged 9 commits into from
Oct 26, 2020

Conversation

shilman
Copy link
Member

@shilman shilman commented Oct 24, 2020

Issue: #12881

What I did

Hack to add the automatic runtime per the NextJS implementation

How to test

Added it by hand in a copy of examples/react-ts's node_modules 🙈

==> Unfortunately it's getting Storybook's React version, not the user's version...

Copy link
Contributor

@tooppaaa tooppaaa left a comment

Choose a reason for hiding this comment

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

🚀 Coudn't do it better


export async function babelDefault(config: TransformOptions) {
const presetReactOptions = hasJsxRuntime() ? { runtime: 'automatic' } : {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't a better approach be to use path.dirname(config.filename) to get the directory for the file being transformed, pass that to hasJsxRuntime, and use it as the paths value for the require.resolve lookup (rather than poking into an arbitrary nm folder)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely ! We have this behavior in other places that needs to be replaced.
I'm interested in creating a @storybook/paths internal package or similar stuff to unify how Storybook infers with client's setup. We had a brief chat with @mrmckeb @shilman @yannbf about it :)

Comment on lines +26 to +32
const context = storybookReactDirName.includes('node_modules')
? path.join(storybookReactDirName, '../../') // Real life case, already in node_modules
: path.join(storybookReactDirName, '../../node_modules'); // SB Monorepo

const hasJsxRuntime = () => {
try {
require.resolve('react/jsx-runtime', { paths: [context] });
Copy link

Choose a reason for hiding this comment

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

Why does it try to use node_modules folder for lookup? It can't work with yarn 2 pnp.

CRA uses require.resolve('react/jsx-runtime'); for the detection:
https://github.com/facebook/create-react-app/blob/282c03f9525fdf8061ffa1ec50dce89296d916bd/packages/react-scripts/config/webpack.config.js#L79-L85

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants