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

Core: Pre-bundle @storybook/theming to avoid emotion11 conflicts #17000

Merged
merged 47 commits into from
Feb 1, 2022

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Dec 13, 2021

Issue: #16716 #13145 #13114

introduce pre-bundle script as an alternative to prepare script.

pre-bundling means we'll:

  • create a runtime bundle of the entire package.
    we actually create the bundle 3 times (CJS, ESM, modern)
  • create a collected type-definitions file

We can specify our source-code entry-point in our package's package.json file:

  "bundlerEntrypoint": "./src/index.ts",

And we can specify which dependencies we'd like to NOT be bundled (will apply to both the runtime bundles and type-definitions) by adding those modules to the dependencies and peerDependencies properties in package.json

To use the prebundle script instead of the prepare script:

  "scripts": {
    "prepare": "ts-node ../../scripts/prebundle.ts"
  },

You may need to install ts-node in the package for this to work.

TODO:

  • fix bootstrap
  • fix linting
  • fix tests
  • investigate if I can generate dist/ts3.4 again.
  • implement some sort of watch mode for yarn dev

@nx-cloud
Copy link

nx-cloud bot commented Dec 13, 2021

☁️ Nx Cloud Report

CI ran the following commands for commit be09c8f. 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.

@nx-cloud
Copy link

nx-cloud bot commented Dec 13, 2021

Nx Cloud Report

We didn't find any information for the current pull request with the commit 603908d.
You might need to set the 'NX_BRANCH' environment variable in your CI pipeline.

Check the Nx Cloud Github Integration documentation for more information.


Sent with 💌 from NxCloud.

@ndelangen ndelangen self-assigned this Dec 13, 2021
@ndelangen ndelangen requested a review from shilman December 13, 2021 15:28
@ndelangen ndelangen marked this pull request as ready for review January 18, 2022 19:00
@ndelangen ndelangen changed the base branch from next to tech/lockfile-regen January 19, 2022 14:55
lib/theming/package.json Outdated Show resolved Hide resolved
lib/theming/tsconfig.json Outdated Show resolved Hide resolved
scripts/prebundle.ts Outdated Show resolved Hide resolved
# Conflicts:
#	lib/theming/package.json
#	yarn.lock
@shilman shilman changed the title pre-bundling certain packages for performance Core: Pre-bundle theming to avoid emotion11 conflicts Feb 1, 2022
@shilman shilman added dependencies and removed performance issue maintenance User-facing maintenance tasks labels Feb 1, 2022
@shilman shilman merged commit ae28437 into next Feb 1, 2022
@shilman shilman deleted the tech/prebundle-wip branch February 1, 2022 13:06
@shilman shilman changed the title Core: Pre-bundle theming to avoid emotion11 conflicts Core: Pre-bundle @storybook/theming to avoid emotion11 conflicts Feb 1, 2022
@shilman shilman added maintenance User-facing maintenance tasks and removed dependencies labels Feb 1, 2022
nathggns added a commit to hashintel/hash that referenced this pull request Mar 11, 2022
Now using alpha storybook for emotion 11 support, which is required for MUI

See mui/material-ui#24282 and storybookjs/storybook#17000
nathggns added a commit to hashintel/hash that referenced this pull request Mar 14, 2022
Now using alpha storybook for emotion 11 support, which is required for MUI

See mui/material-ui#24282 and storybookjs/storybook#17000
nathggns added a commit to hashintel/hash that referenced this pull request Mar 14, 2022
Now using alpha storybook for emotion 11 support, which is required for MUI

See mui/material-ui#24282 and storybookjs/storybook#17000
nathggns added a commit to hashintel/hash that referenced this pull request Mar 29, 2022
Now using alpha storybook for emotion 11 support, which is required for MUI

See mui/material-ui#24282 and storybookjs/storybook#17000
itkrt2y added a commit to itkrt2y/chakra-ui that referenced this pull request Jun 1, 2022
itkrt2y added a commit to itkrt2y/chakra-ui that referenced this pull request Jun 5, 2022
@robinmetral
Copy link

This mean that the emotionAlias flag can be dropped, right?

emotionAlias: false, // TODO remove in 7.0, this no longer does anything

@shilman
Copy link
Member

shilman commented Jun 16, 2022

@robinmetral already done in the 7.0 branch future/base

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.

5 participants