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: Make prettier an optional peer dependency #29223

Merged

Conversation

JReinhold
Copy link
Contributor

@JReinhold JReinhold commented Sep 26, 2024

Closes #29084

What I did

  1. Made prettier an optional peer dependency of @storybook/core. The Save from Controls that is using Prettier to format the output according to the project's options, is already written to support environments where Prettier isn't installed.

    async function getPrettier() {
    return import('prettier').catch((e) => ({
    resolveConfig: async () => null,
    format: (content: string) => content,
    }));
    }

  2. kept prettier as an internal for the @storybook/core/components entrypoint, because the syntax highlighter uses it when formatting is enabled. It uses the lighter weight prettier/standalone and html plugin.

https://github.com/storybookjs/storybook/blob/d3f5ad98982d4320fde6dac9c901440d48496be8/code/core/src/components/components/syntaxhighlighter/formatter.ts

(Given that it only supports formatting html and not js/jsx, etc. I question how valuable it is. I've added a bullet in the 9.0 changes to consider removing it)

Before

image

image

After

image

image

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

  1. install the canary version in a separate sandbox to ensure prettier isn't polluted into the project.
  2. ensure prettier isn't in the project
  3. start storybook, use the save from controls feature, test that the output is written to the file without any issues. (new component, update existing story, save new story) give the file some very bad formatting and see that the changes we write are still okay
  4. install prettier
  5. repeat 3, see that the story files are now formatted

to test formatting in the syntax highlighter:

  1. ensure prettier is not in the project
  2. add the following MDX file to your storybook:
import { Source } from '@storybook/blocks';

# testing Source

<Source format="html" code={`<body><div>hello</div></body>`} />
  1. ensure that it is properly formatted in the storybook.

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This pull request has been released as version 0.0.0-pr-29223-sha-d3f5ad98. Try it out in a new sandbox by running npx storybook@0.0.0-pr-29223-sha-d3f5ad98 sandbox or in an existing project with npx storybook@0.0.0-pr-29223-sha-d3f5ad98 upgrade.

More information
Published version 0.0.0-pr-29223-sha-d3f5ad98
Triggered by @JReinhold
Repository storybookjs/storybook
Branch 29084-minimize-the-bundling-of-prettier-in-storybookcore
Commit d3f5ad98
Datetime Thu Sep 26 20:49:55 UTC 2024 (1727383795)
Workflow run 11060034306

To request a new release of this pull request, mention the @storybookjs/core team.

core team members can create a new canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=29223

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 77.5 MB 77.7 MB 202 kB 4.29 0.3%
initSize 162 MB 152 MB -10 MB -89.17 🔰-6.6%
diffSize 84.8 MB 74.6 MB -10.2 MB -86.53 🔰-13.7%
buildSize 6.96 MB 6.96 MB 0 B -2.08 0%
buildSbAddonsSize 1.57 MB 1.57 MB 0 B -2.38 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.91 MB 1.91 MB 0 B -1.83 0%
buildSbPreviewSize 311 kB 311 kB 0 B -2.38 0%
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.98 MB 3.98 MB 0 B -2.03 0%
buildPreviewSize 2.97 MB 2.97 MB 0 B -2.38 0%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 15.6s 7.3s -8s -253ms -0.64 -112.3%
generateTime 20.8s 24.4s 3.5s 0.45 14.5%
initTime 16.3s 16.4s 126ms -0.15 0.8%
buildTime 8.6s 9s 346ms -1.49 3.8%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 6.3s 6.3s -48ms -0.91 -0.8%
devManagerResponsive 4.2s 4.1s -65ms -1.13 -1.6%
devManagerHeaderVisible 521ms 527ms 6ms -1.85 1.1%
devManagerIndexVisible 548ms 564ms 16ms -1.73 2.8%
devStoryVisibleUncached 970ms 1.1s 166ms -0.5 14.6%
devStoryVisible 549ms 562ms 13ms -1.81 2.3%
devAutodocsVisible 493ms 530ms 37ms -1.01 7%
devMDXVisible 478ms 464ms -14ms -1.47 -3%
buildManagerHeaderVisible 520ms 497ms -23ms -1.6 -4.6%
buildManagerIndexVisible 558ms 500ms -58ms -1.68 🔰-11.6%
buildStoryVisible 594ms 537ms -57ms -2.02 🔰-10.6%
buildAutodocsVisible 442ms 442ms 0ms -1.87 0%
buildMDXVisible 438ms 459ms 21ms -1.43 4.6%

Greptile Summary

This PR makes Prettier an optional peer dependency in @storybook/core and @storybook/cli, while retaining it as an internal dependency for specific components.

  • Modified code/core/package.json to make Prettier an optional peer dependency
  • Updated code/core/scripts/entries.ts to include Prettier as an internal dependency for the syntax highlighter component
  • Added Prettier as an optional peer dependency in code/lib/cli/package.json
  • Reduces bundle size for users who don't need Prettier functionality
  • Maintains Prettier support for code formatting in specific components like the syntax highlighter

@JReinhold JReinhold linked an issue Sep 26, 2024 that may be closed by this pull request
Copy link

nx-cloud bot commented Sep 26, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 41d8e09. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@JReinhold JReinhold added components core ci:normal maintenance User-facing maintenance tasks labels Sep 26, 2024
@JReinhold
Copy link
Contributor Author

I've tested this with npm, Yarn PnP and pnpm

@JReinhold JReinhold marked this pull request as ready for review September 27, 2024 12:27
@JReinhold JReinhold self-assigned this Sep 27, 2024
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

LGTM

3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@JReinhold JReinhold merged commit 5d14706 into next Sep 27, 2024
57 checks passed
@JReinhold JReinhold deleted the 29084-minimize-the-bundling-of-prettier-in-storybookcore branch September 27, 2024 13:03
@github-actions github-actions bot mentioned this pull request Sep 27, 2024
10 tasks
@JReinhold JReinhold restored the 29084-minimize-the-bundling-of-prettier-in-storybookcore branch October 3, 2024 18:54
@JReinhold JReinhold added the needs qa Indicates that this needs manual QA during the upcoming minor/major release label Oct 24, 2024
@news-manishpatel
Copy link

We use prettier v1 and our builds started to break. Should this have been in a major release?

@JReinhold
Copy link
Contributor Author

@news-manishpatel could you elaborate a bit more on how your builds are starting to break?

@news-manishpatel
Copy link

@JReinhold We use prettier v1 and with storybook v8.3.x this works fine but with 8.4.x this breaks.

image

@ndelangen
Copy link
Member

@JReinhold should we broaden the version range to include prettier v1?

ndelangen added a commit that referenced this pull request Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:normal components core maintenance User-facing maintenance tasks needs qa Indicates that this needs manual QA during the upcoming minor/major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minimize the bundling of prettier in @storybook/core
3 participants