Core: Replace `fs-extra` with the native APIs by pralkarz · Pull Request #29126 · storybookjs/storybook

pralkarz · 2024-09-14T12:07:23Z

Closes #29104. Works on #29038.

What I did

This PR replaces fs-extra with the native APIs in all code/**/* packages. Now it's only included in scripts/ and as a transitive dependency.

Checklist for Contributors

Testing

I've confirmed that the unit tests pass locally in the packages I've made the changes to. Please advise on which integration/E2E tests I should run (if any).

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

stories
unit tests
integration tests
end-to-end tests

Manual testing

Here's what I did for most changes (in packages that have had the unit tests):

Run npx vitest in the package directory before making any changes.
Build in watch mode.
Make changes and either rerun npx vitest after every change (for smaller packages) or utilize Vitest's watching capabilities (for bigger packages).

As above, please advise if I should do some more testing here.

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 PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

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

name	before	after	diff	z	%
createSize	0 B	0 B	0 B	-	-
generateSize	77.4 MB	77.4 MB	0 B	2.22	0%
initSize	163 MB	162 MB	-507 kB	-0.23	-0.3%
diffSize	85.4 MB	84.9 MB	-507 kB	-0.27	-0.6%
buildSize	7.57 MB	7.57 MB	-2 B	0.8	0%
buildSbAddonsSize	1.66 MB	1.66 MB	0 B	0.89	0%
buildSbCommonSize	195 kB	195 kB	0 B	-	0%
buildSbManagerSize	2.34 MB	2.34 MB	0 B	0.82	0%
buildSbPreviewSize	352 kB	352 kB	0 B	0.65	0%
buildStaticSize	0 B	0 B	0 B	-	-
buildPrebuildSize	4.55 MB	4.55 MB	0 B	0.89	0%
buildPreviewSize	3.02 MB	3.02 MB	-2 B	0.11	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	25.4s	13s	-12s -412ms	0.03	-95.3%
generateTime	19.7s	20.6s	900ms	-0.13	4.4%
initTime	16.1s	18.8s	2.7s	1.33	🔺14.7%
buildTime	10.4s	10.5s	146ms	-0.88	1.4%
testBuildTime	0ms	0ms	0ms	-	-
devPreviewResponsive	7.9s	7.7s	-277ms	0.82	-3.6%
devManagerResponsive	5.1s	5.2s	18ms	1.23	0.3%
devManagerHeaderVisible	874ms	771ms	-103ms	-0.37	-13.4%
devManagerIndexVisible	915ms	802ms	-113ms	-0.41	-14.1%
devStoryVisibleUncached	1.3s	1.3s	26ms	-0.09	1.9%
devStoryVisible	914ms	801ms	-113ms	-0.42	-14.1%
devAutodocsVisible	776ms	671ms	-105ms	-0.55	-15.6%
devMDXVisible	778ms	679ms	-99ms	-0.43	-14.6%
buildManagerHeaderVisible	904ms	720ms	-184ms	-0.57	-25.6%
buildManagerIndexVisible	944ms	721ms	-223ms	-0.74	-30.9%
buildStoryVisible	945ms	834ms	-111ms	0.03	-13.3%
buildAutodocsVisible	898ms	676ms	-222ms	-0.42	-32.8%
buildMDXVisible	726ms	679ms	-47ms	-0.06	-6.9%

Greptile Summary

This PR replaces the fs-extra package with native Node.js fs and fs/promises APIs across multiple files in the Storybook codebase, aiming to reduce dependencies and use built-in functionality.

Replaced fs-extra with node:fs and node:fs/promises in core packages and builders
Updated file operations to use native methods like readFile, writeFile, cp, and rm
Removed fs-extra from package.json files in affected packages
Added TODO comments regarding experimental fsPromises.cp usage in Node 16-21
Implemented custom file existence checks to replace fs-extra's ensureFile functionality

VS Code complains about not being able to find Node types, but building the package works. Need to investigate.

code/builders/builder-vite/src/index.ts

greptile-apps

_{30 file(s) reviewed, 10 comment(s)}
_{Edit PR Review Bot Settings}

code/__mocks__/fs.ts

greptile-apps · 2024-09-14T18:14:30Z

code/__mocks__/fs.ts

+  mockFiles = newMockFiles;
+}
+
+export const readFileSync = (filePath = '') => mockFiles[filePath];


logic: Default parameter value '' might cause unexpected behavior if undefined is passed

code/__mocks__/fs/promises.ts

code/builders/builder-vite/src/plugins/external-globals-plugin.ts

greptile-apps · 2024-09-14T18:19:48Z

code/core/scripts/helpers/dependencies.ts

+      const { dependencies = {}, peerDependencies = {} } = JSON.parse(
+        await readFile(path, { encoding: 'utf-8' })
+      );


style: Consider wrapping JSON.parse in a try-catch block to handle potential JSON parsing errors

code/core/scripts/helpers/dependencies.ts

code/core/scripts/helpers/generateTypesMapperFiles.ts

code/core/src/common/utils/get-storybook-info.ts

code/core/src/core-server/build-static.ts

code/core/src/cli/helpers.ts

pralkarz · 2024-09-16T16:02:50Z

~~The failures seem unrelated to my PR, although I'm not sure since there's not much output. Are they expected, @JReinhold?~~

EDIT: They were related to my PR. 😄

nx-cloud · 2024-09-16T18:16:24Z

☁️ Nx Cloud Report

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

Sent with 💌 from NxCloud.

JReinhold

Fantastic job!

Just to make sure, have you ensured all of these APIs are available in Node 18, which we currently support?

code/builders/builder-vite/src/plugins/external-globals-plugin.ts

code/core/scripts/helpers/generateTypesMapperFiles.ts

code/core/src/cli/helpers.ts

code/core/src/core-server/build-static.ts

pralkarz · 2024-09-16T21:08:58Z

Just to make sure, have you ensured all of these APIs are available in Node 18, which we currently support?

Yup, everything I've used was introduced before Node 18. The only concern could be fsPromises.cp which is marked as experimental, but that's been resolved in another PR comment.

As for the failing ci/circleci: test-portable-stories-nextjs: I think it might be flaky? Sometimes it fails with the same error as now, but it passed before merging next.

JReinhold · 2024-09-16T21:14:00Z

Yes it looks flaky, don't worry about it.

ziebam added 23 commits September 14, 2024 00:17

Remove fs-extra from presets/server

247efff

Remove fs-extra from presets/react-webpack

2ac99b3

Remove fs-extra from lib/create-storybook

ce73ddb

Remove fs-extra from lib/cli-storybook

5ed3ab4

Remove fs-extra from frameworks/nextjs

2e89262

Remove fs-extra from builders/builder-webpack5

b080e71

Remove fs-extra from builders/builder-vite

b9c600a

Remove fs-extra from addons/docs

67dbe11

Remove rogue import in builders/builder-vite

cf80eaa

Merge 'upstream/next' into 'remove-fs-extra'

ef33853

Remove rogue comment

353dfb1

Remove fs-extra from presets/server-webpack

08c6eef

Remove fs-extra from renderers/svelte

a09abbc

VS Code complains about not being able to find Node types, but building the package works. Need to investigate.

Remove fs-extra from addons/links

93b866b

VS Code complains about not being able to find Node types, but building the package works. Need to investigate.

core: Remove fs-extra from src/telemetry

7c14a13

core: Remove fs-extra from src/common/utils

7816541

core: Remove fs-extra from src/core-server/utils

fb2455b

Add recursive: true to cp calls

2dcc7b7

core: Remove fs-extra from src/core-server

ca39982

Close the file descriptor

a42ff7e

core: Remove fs-extra from src/builder-manager

174868a

core: Remove fs-extra from scripts

643acbf

Remove fs-extra from code

89cdff3

pralkarz marked this pull request as ready for review September 14, 2024 18:13

pralkarz commented Sep 14, 2024

View reviewed changes

code/builders/builder-vite/src/index.ts Outdated Show resolved Hide resolved

greptile-apps bot reviewed Sep 14, 2024

View reviewed changes

43081j reviewed Sep 15, 2024

View reviewed changes

code/core/src/cli/helpers.ts Show resolved Hide resolved

Replace 'utf-8' with 'utf8' for consistency with Node docs

0d10b95

pralkarz mentioned this pull request Sep 15, 2024

Transitive fs-extra dependencies in storybook e18e/ecosystem-issues#113

Open

11 tasks

JReinhold self-assigned this Sep 16, 2024

JReinhold added build Internal-facing build tooling & test updates ci:normal labels Sep 16, 2024

JReinhold mentioned this pull request Sep 16, 2024

[Tracking] Reduce install footprint 🐾 #29038

Closed

89 tasks

Merge branch 'next' into remove-fs-extra

f5b1525

ziebam added 4 commits September 16, 2024 20:25

Remove the TODOs regarding fsPromises.cp

8346b85

Rewrite how ensureFile is replaced

f0593f9

Remove fs-extra from lib/cli/scripts

ea609bf

Fix the emptyDir + ensureDir replacement

b034203

JReinhold reviewed Sep 16, 2024

View reviewed changes

Merge branch 'next' into remove-fs-extra

8ac3e53

JReinhold approved these changes Sep 16, 2024

View reviewed changes

remove @types/fs-extra

8115974

JReinhold changed the title ~~Replace fs-extra with the native APIs~~ Core: Replace fs-extra with the native APIs Sep 17, 2024

JReinhold added feature request dependencies and removed build Internal-facing build tooling & test updates feature request labels Sep 17, 2024

JReinhold merged commit ea27938 into storybookjs:next Sep 17, 2024

pralkarz deleted the remove-fs-extra branch September 17, 2024 10:35

github-actions bot mentioned this pull request Sep 17, 2024

Release: Prerelease 8.4.0-alpha.1 #29118

Merged

21 tasks

benmccann mentioned this pull request Oct 3, 2024

docs: add september showcase post e18e/e18e#30

Merged

This was referenced Oct 15, 2024

[Bug]: [8.4.0-alpha.7] index.html is deleted when storybook is built twice #29358

Closed

Core: Fix building Storybook deleting project root files #29371

Merged

Uh oh!

Conversation

pralkarz commented Sep 14, 2024 • edited by storybook-app-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

Checklist for Contributors

Testing

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

Manual testing

Documentation

Checklist for Maintainers

🦋 Canary release

Greptile Summary

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot Sep 14, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot Sep 14, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pralkarz commented Sep 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nx-cloud bot commented Sep 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

☁️ Nx Cloud Report

Uh oh!

JReinhold left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pralkarz commented Sep 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JReinhold commented Sep 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pralkarz commented Sep 14, 2024 •

edited by storybook-app-bot bot

Loading

pralkarz commented Sep 16, 2024 •

edited

Loading

nx-cloud bot commented Sep 16, 2024 •

edited

Loading

pralkarz commented Sep 16, 2024 •

edited

Loading