JReinhold commented 1 month ago

Closes #29084

What I did

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. https://github.com/storybookjs/storybook/blob/d3f5ad98982d4320fde6dac9c901440d48496be8/code/core/src/common/utils/formatter.ts#L1-L6
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

After

Checklist for Contributors

Testing

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

[ ] stories
[ ] unit tests
[ ] integration tests
[x] 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!

install the canary version in a separate sandbox to ensure prettier isn't polluted into the project.
ensure prettier isn't in the project
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
install prettier
repeat 3, see that the story files are now formatted

to test formatting in the syntax highlighter:

ensure prettier is not in the project
add the following MDX file to your storybook:

import { Source } from '@storybook/blocks';

# testing Source

<Source format="html" code={`<body><div>hello</div></body>`} />

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`](https://npmjs.com/package/storybook/v/0.0.0-pr-29223-sha-d3f5ad98) | | **Triggered by** | @JReinhold | | **Repository** | [storybookjs/storybook](https://github.com/storybookjs/storybook) | | **Branch** | [`29084-minimize-the-bundling-of-prettier-in-storybookcore`](https://github.com/storybookjs/storybook/tree/29084-minimize-the-bundling-of-prettier-in-storybookcore) | | **Commit** | [`d3f5ad98`](https://github.com/storybookjs/storybook/commit/d3f5ad98982d4320fde6dac9c901440d48496be8) | | **Datetime** | Thu Sep 26 20:49:55 UTC 2024 (`1727383795`) | | **Workflow run** | [11060034306](https://github.com/storybookjs/storybook/actions/runs/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](https://github.com/storybookjs/storybook/actions/workflows/canary-release-pr.yml) 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

nx-cloud[bot] commented 1 month ago

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 41d8e095fd44869d0b6399458638d7fcac31a825. 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

- [`nx run-many -t build --parallel=3`](https://cloud.nx.app/runs/eS70R6P1W4?utm_source=pull-request&utm_medium=comment)

Sent with 💌 from NxCloud.

JReinhold commented 1 month ago

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

news-manishpatel commented 2 weeks ago

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

JReinhold commented 2 weeks ago

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