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

UI: Prebundle @storybook/components #17304

Merged
merged 26 commits into from
Feb 4, 2022
Merged

UI: Prebundle @storybook/components #17304

merged 26 commits into from
Feb 4, 2022

Conversation

ndelangen
Copy link
Member

This uses #17000 to prebundle lib/components

@nx-cloud
Copy link

nx-cloud bot commented Jan 20, 2022

☁️ Nx Cloud Report

CI ran the following commands for commit 8dcd346. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@ndelangen ndelangen self-assigned this Jan 20, 2022
@ndelangen ndelangen added ci: do not merge maintenance User-facing maintenance tasks labels Jan 20, 2022
Base automatically changed from tech/bundling-ui to next February 1, 2022 15:28
# Conflicts:
#	lib/components/package.json
@ndelangen
Copy link
Member Author

hmmm..

export 'mdx' was not found in '@mdx-js/react'

@ndelangen
Copy link
Member Author

@shilman any idea on the above?

@shilman
Copy link
Member

shilman commented Feb 1, 2022

@ndelangen looks like it's referenced here:

https://github.com/storybookjs/storybook/blob/next/addons/docs/jest-transform-mdx.js#L15

And looks like it's defined in the package:

exports.mdx = createElement;

So I'm not sure why it's failing 🤷‍♂️

@shilman
Copy link
Member

shilman commented Feb 1, 2022

@ndelangen aha here's why it's failing: https://github.com/mdx-js/mdx/releases/tag/2.0.0

something is installing 2.0 which was released 2h ago, and we're compatible with 1.x only.

@ndelangen
Copy link
Member Author

but we specify we need 1.x:

"@mdx-js/react": "^1.6.22",

@ndelangen
Copy link
Member Author

ndelangen commented Feb 1, 2022

ahh maybe this is is?

const yarn2Dependencies =
packageManager.type === 'yarn2' ? ['@storybook/addon-docs', '@mdx-js/react'] : [];

I think @mdx-js/react should be removed here. Or set to 1.x version

@ndelangen
Copy link
Member Author

getting the same problem here:
#17375

@ndelangen
Copy link
Member Author

@shilman This might be a pretty big deal!!

@shilman shilman changed the title prebunde lib/component UI: Prebundle @storybook/components Feb 2, 2022
@ndelangen
Copy link
Member Author

I had to add @types/react to angular to get it to compile... @gaetanmaisse WDYT about this?

@ndelangen
Copy link
Member Author

Hmm, so that didn't work?

ERR! => Failed to build the preview
ERR! node_modules/@storybook/router/dist/ts3.9/_modules/react-router-dom-index.d.ts:57:20 - error TS2694: Namespace 'React' has no exported member 'HTMLAttributeAnchorTarget'.
ERR! 
ERR! 57     target?: React.HTMLAttributeAnchorTarget;
ERR!                       ~~~~~~~~~~~~~~~~~~~~~~~~~
ERR!

@ndelangen
Copy link
Member Author

If I added @types/react to the packages (router, theming) wouldn't that potentially cause issues for Vue?

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Great job @ndelangen 🎖️

@shilman shilman merged commit 1f72bf9 into next Feb 4, 2022
@shilman shilman deleted the tech/bundling-components branch February 4, 2022 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants