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

Make @storybook/csf-tools a peer dependency #874

Closed
wants to merge 5 commits into from

Conversation

tmeasday
Copy link
Member

@tmeasday tmeasday commented Dec 12, 2023

Externalize @storybook/csf-tools which is a large dependency which will have been by installed by the user already (see this conversation: https://chromaticqa.slack.com/archives/CMMBWSJ1W/p1702342681865589).

We only use that package for metadata extraction in SB6 so we should verify this doesn't break that.

📦 Published PR as canary version: 11.0.0--canary.874.7204664733.0

✨ Test out this PR locally via:

npm install chromatic@11.0.0--canary.874.7204664733.0
# or 
yarn add chromatic@11.0.0--canary.874.7204664733.0

@shilman
Copy link
Member

shilman commented Dec 12, 2023

We should probably try to optimize this inside csf-tools somehow. It's meant to be a lightweight utility that anybody can use to read/modify/write CSF but it's pretty chunky.

@valentinpalkovic and i have discussed rewriting in SWC and I'm not sure whether that would help or hurt the install size.

@tmeasday tmeasday changed the title Use code splitting in tsup CJS output Make @storybook/csf-tools a peer dependency Dec 12, 2023
@tmeasday
Copy link
Member Author

tmeasday commented Dec 13, 2023

@ghengeveld we should decide whether we want to make this change.

Upside: our package is ~1.15MB smaller (main chunk 3.79 MB -> 2.64 MB)
Downside: if the user is using SB6 & a strict package manager (like yarn pnp or pnpm), they will need an explicit dependency on @storybook/csf-tools (matching version with their other SB packages) in package.json.

I made a change to only import async so that the above issue will only affect such package manager users if they are on SB6 (where we actually use the code). We'll also need to figure out how to document this.

Base automatically changed from tom/split-dist-output to main December 14, 2023 04:33
@tmeasday tmeasday added major Auto: Increment the major version when merged skip-release Auto: Preserve the current version when merged labels Dec 14, 2023
@tmeasday tmeasday added release Auto: Create a `latest` release when merged and removed skip-release Auto: Preserve the current version when merged labels Dec 14, 2023
@ndelangen
Copy link
Member

  • Why is the big dependency a problem?
  • Could we make it a seperate entrypoint and async load it that way, so it's only loaded when it's needed that way?
  • Could we release a version of chromatic's CLI that only supports storybook 7 and up?

@tmeasday
Copy link
Member Author

After some consideration we decided not to do this an instead reduce the size of @storybook/csf-tools if possible. Although we can almost certainly rely on the user having that package already, it's not strictly guaranteed, so relying on it is too dangerous.

@tmeasday tmeasday closed this Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major Auto: Increment the major version when merged release Auto: Create a `latest` release when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants