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: Replace fs-extra with the native APIs #29126

Merged
merged 31 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
247efff
Remove `fs-extra` from `presets/server`
ziebam Sep 13, 2024
2ac99b3
Remove `fs-extra` from `presets/react-webpack`
ziebam Sep 13, 2024
ce73ddb
Remove `fs-extra` from `lib/create-storybook`
ziebam Sep 14, 2024
5ed3ab4
Remove `fs-extra` from `lib/cli-storybook`
ziebam Sep 14, 2024
2e89262
Remove `fs-extra` from `frameworks/nextjs`
ziebam Sep 14, 2024
b080e71
Remove `fs-extra` from `builders/builder-webpack5`
ziebam Sep 14, 2024
b9c600a
Remove `fs-extra` from `builders/builder-vite`
ziebam Sep 14, 2024
67dbe11
Remove `fs-extra` from `addons/docs`
ziebam Sep 14, 2024
cf80eaa
Remove rogue import in `builders/builder-vite`
ziebam Sep 14, 2024
ef33853
Merge 'upstream/next' into 'remove-fs-extra'
ziebam Sep 14, 2024
353dfb1
Remove rogue comment
ziebam Sep 14, 2024
08c6eef
Remove `fs-extra` from `presets/server-webpack`
ziebam Sep 14, 2024
a09abbc
Remove `fs-extra` from `renderers/svelte`
ziebam Sep 14, 2024
93b866b
Remove `fs-extra` from `addons/links`
ziebam Sep 14, 2024
7c14a13
core: Remove `fs-extra` from `src/telemetry`
ziebam Sep 14, 2024
7816541
core: Remove `fs-extra` from `src/common/utils`
ziebam Sep 14, 2024
fb2455b
core: Remove `fs-extra` from `src/core-server/utils`
ziebam Sep 14, 2024
2dcc7b7
Add `recursive: true` to `cp` calls
ziebam Sep 14, 2024
ca39982
core: Remove `fs-extra` from `src/core-server`
ziebam Sep 14, 2024
a42ff7e
Close the file descriptor
ziebam Sep 14, 2024
174868a
core: Remove `fs-extra` from `src/builder-manager`
ziebam Sep 14, 2024
643acbf
core: Remove `fs-extra` from `scripts`
ziebam Sep 14, 2024
89cdff3
Remove `fs-extra` from `code`
ziebam Sep 14, 2024
0d10b95
Replace 'utf-8' with 'utf8' for consistency with Node docs
ziebam Sep 15, 2024
f5b1525
Merge branch 'next' into remove-fs-extra
JReinhold Sep 16, 2024
8346b85
Remove the TODOs regarding `fsPromises.cp`
ziebam Sep 16, 2024
f0593f9
Rewrite how `ensureFile` is replaced
ziebam Sep 16, 2024
ea609bf
Remove `fs-extra` `from lib/cli/scripts`
ziebam Sep 16, 2024
b034203
Fix the `emptyDir` + `ensureDir` replacement
ziebam Sep 16, 2024
8ac3e53
Merge branch 'next' into remove-fs-extra
JReinhold Sep 16, 2024
8115974
remove @types/fs-extra
JReinhold Sep 17, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 0 additions & 29 deletions code/__mocks__/fs.js

This file was deleted.

32 changes: 32 additions & 0 deletions code/__mocks__/fs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { vi } from 'vitest';

// This is a custom function that our tests can use during setup to specify
// what the files on the "mock" filesystem should look like when any of the
// `fs` APIs are used.
let mockFiles = Object.create(null);
JReinhold marked this conversation as resolved.
Show resolved Hide resolved

// eslint-disable-next-line no-underscore-dangle, @typescript-eslint/naming-convention
export function __setMockFiles(newMockFiles: Record<string, string | null>) {
mockFiles = newMockFiles;
}

export const readFileSync = (filePath = '') => mockFiles[filePath];
Copy link
Contributor

Choose a reason for hiding this comment

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

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

export const existsSync = (filePath: string) => !!mockFiles[filePath];
export const lstatSync = (filePath: string) => ({
isFile: () => !!mockFiles[filePath],
});
export const realpathSync = vi.fn();
export const readdir = vi.fn();
export const readdirSync = vi.fn();
export const readlinkSync = vi.fn();

export default {
__setMockFiles,
readFileSync,
existsSync,
lstatSync,
realpathSync,
readdir,
readdirSync,
readlinkSync,
};
32 changes: 32 additions & 0 deletions code/__mocks__/fs/promises.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { vi } from 'vitest';

// This is a custom function that our tests can use during setup to specify
// what the files on the "mock" filesystem should look like when any of the
// `fs` APIs are used.
let mockFiles = Object.create(null);
JReinhold marked this conversation as resolved.
Show resolved Hide resolved

// eslint-disable-next-line no-underscore-dangle, @typescript-eslint/naming-convention
export function __setMockFiles(newMockFiles: Record<string, string | null>) {
mockFiles = newMockFiles;
}

export const writeFile = vi.fn(async (filePath: string, content: string) => {
mockFiles[filePath] = content;
});
export const readFile = vi.fn(async (filePath: string) => mockFiles[filePath]);
ziebam marked this conversation as resolved.
Show resolved Hide resolved
export const lstat = vi.fn(async (filePath: string) => ({
isFile: () => !!mockFiles[filePath],
}));
export const readdir = vi.fn();
export const readlink = vi.fn();
export const realpath = vi.fn();

export default {
__setMockFiles,
writeFile,
readFile,
lstat,
readdir,
readlink,
realpath,
};
1 change: 0 additions & 1 deletion code/addons/docs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@
"@storybook/global": "^5.0.0",
"@storybook/react-dom-shim": "workspace:*",
"@types/react": "^16.8.0 || ^17.0.0 || ^18.0.0",
"fs-extra": "^11.1.0",
"react": "^16.8.0 || ^17.0.0 || ^18.0.0",
"react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0",
"rehype-external-links": "^3.0.0",
Expand Down
1 change: 0 additions & 1 deletion code/addons/links/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@
"ts-dedent": "^2.0.0"
},
"devDependencies": {
"fs-extra": "^11.1.0",
"typescript": "^5.3.2"
},
"peerDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion code/addons/links/scripts/fix-preview-api-reference.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { readFile, writeFile } from 'fs-extra';
import { readFile, writeFile } from 'node:fs/promises';

/* I wish this wasn't needed..
* There seems to be some bug in tsup / the unlaying lib that does DTS bundling
Expand Down
1 change: 0 additions & 1 deletion code/builders/builder-vite/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
"es-module-lexer": "^1.5.0",
"express": "^4.19.2",
"find-cache-dir": "^3.0.0",
"fs-extra": "^11.1.0",
"magic-string": "^0.30.0",
"ts-dedent": "^2.0.0"
},
Expand Down
14 changes: 8 additions & 6 deletions code/builders/builder-vite/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
// noinspection JSUnusedGlobalSymbols
import { cp, readFile } from 'node:fs/promises';
import { join, parse } from 'node:path';

import { NoStatsForViteDevError } from 'storybook/internal/server-errors';
import type { Options } from 'storybook/internal/types';

import type { RequestHandler } from 'express';
import express from 'express';
import * as fs from 'fs-extra';
import { corePath } from 'storybook/core-path';
import type { ViteDevServer } from 'vite';

Expand Down Expand Up @@ -34,10 +34,9 @@ function iframeMiddleware(options: Options, server: ViteDevServer): RequestHandl
return;
}

const indexHtml = await fs.readFile(
require.resolve('@storybook/builder-vite/input/iframe.html'),
'utf-8'
);
const indexHtml = await readFile(require.resolve('@storybook/builder-vite/input/iframe.html'), {
encoding: 'utf-8',
});
const generated = await transformIframeHtml(indexHtml, options);
const transformed = await server.transformIndexHtml('/iframe.html', generated);
res.setHeader('Content-Type', 'text/html');
Expand Down Expand Up @@ -85,14 +84,17 @@ export const build: ViteBuilder['build'] = async ({ options }) => {
const previewDirOrigin = previewResolvedDir;
const previewDirTarget = join(options.outputDir || '', `sb-preview`);

const previewFiles = fs.copy(previewDirOrigin, previewDirTarget, {
// TODO: `fsPromises.cp` is marked as experimental in Node 16-21. Ask in the PR whether we should
// use it anyway or stick to `fs-extra` for now.
ziebam marked this conversation as resolved.
Show resolved Hide resolved
const previewFiles = cp(previewDirOrigin, previewDirTarget, {
filter: (src) => {
const { ext } = parse(src);
if (ext) {
return ext === '.js';
}
return true;
},
recursive: true,
});

const [out] = await Promise.all([viteCompilation, previewFiles]);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { closeSync, existsSync, openSync } from 'node:fs';
import { writeFile } from 'node:fs/promises';
ziebam marked this conversation as resolved.
Show resolved Hide resolved
import { join } from 'node:path';

import { init, parse } from 'es-module-lexer';
import findCacheDirectory from 'find-cache-dir';
import { ensureFile, writeFile } from 'fs-extra';
import MagicString from 'magic-string';
import type { Alias, Plugin } from 'vite';

Expand Down Expand Up @@ -59,7 +60,9 @@ export async function externalGlobalsPlugin(externals: Record<string, string>) {
(Object.keys(externals) as Array<keyof typeof externals>).map(async (externalKey) => {
const externalCachePath = join(cachePath, `${externalKey}.js`);
newAlias.push({ find: new RegExp(`^${externalKey}$`), replacement: externalCachePath });
await ensureFile(externalCachePath);
if (!existsSync(externalCachePath)) {
closeSync(openSync(externalCachePath, 'w'));
}
ziebam marked this conversation as resolved.
Show resolved Hide resolved
await writeFile(externalCachePath, `module.exports = ${externals[externalKey]};`);
})
);
Expand Down
1 change: 0 additions & 1 deletion code/builders/builder-webpack5/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@
"es-module-lexer": "^1.5.0",
"express": "^4.19.2",
"fork-ts-checker-webpack-plugin": "^8.0.0",
"fs-extra": "^11.1.0",
"html-webpack-plugin": "^5.5.0",
"magic-string": "^0.30.5",
"path-browserify": "^1.0.1",
Expand Down
7 changes: 5 additions & 2 deletions code/builders/builder-webpack5/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { cp } from 'node:fs/promises';
import { join, parse } from 'node:path';

import { PREVIEW_BUILDER_PROGRESS } from 'storybook/internal/core-events';
Expand All @@ -12,7 +13,6 @@ import type { Builder, Options } from 'storybook/internal/types';
import { checkWebpackVersion } from '@storybook/core-webpack';

import express from 'express';
import fs from 'fs-extra';
import prettyTime from 'pretty-hrtime';
import { corePath } from 'storybook/core-path';
import type { Configuration, Stats, StatsOptions } from 'webpack';
Expand Down Expand Up @@ -292,14 +292,17 @@ const builder: BuilderFunction = async function* builderGeneratorFn({ startTime,
const previewDirOrigin = previewResolvedDir;
const previewDirTarget = join(options.outputDir || '', `sb-preview`);

const previewFiles = fs.copy(previewDirOrigin, previewDirTarget, {
// TODO: `fsPromises.cp` is marked as experimental in Node 16-21. Ask in the PR whether we should
// use it anyway or stick to `fs-extra` for now.
const previewFiles = cp(previewDirOrigin, previewDirTarget, {
filter: (src) => {
const { ext } = parse(src);
if (ext) {
return ext === '.js';
}
return true;
},
recursive: true,
});

const [webpackCompilationOutput] = await Promise.all([webpackCompilation, previewFiles]);
Expand Down
2 changes: 0 additions & 2 deletions code/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,6 @@
"@types/diff": "^5.0.9",
"@types/ejs": "^3.1.1",
"@types/find-cache-dir": "^5.0.0",
"@types/fs-extra": "^11.0.1",
"@types/js-yaml": "^4.0.5",
"@types/lodash": "^4.14.167",
"@types/node": "^22.0.0",
Expand Down Expand Up @@ -371,7 +370,6 @@
"find-cache-dir": "^5.0.0",
"find-up": "^7.0.0",
"flush-promises": "^1.0.2",
"fs-extra": "^11.1.0",
"fuse.js": "^3.6.1",
"get-npm-tarball-url": "^2.0.3",
"glob": "^10.0.0",
Expand Down
7 changes: 4 additions & 3 deletions code/core/scripts/helpers/dependencies.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { readFile } from 'node:fs/promises';
import { join } from 'node:path';

import { readJson } from 'fs-extra';

export async function flattenDependencies(
list: string[],
output: string[] = [],
Expand All @@ -18,7 +17,9 @@ export async function flattenDependencies(
console.log(dep + ' not found');
return;
}
const { dependencies = {}, peerDependencies = {} } = await readJson(path);
const { dependencies = {}, peerDependencies = {} } = JSON.parse(
await readFile(path, { encoding: 'utf-8' })
ziebam marked this conversation as resolved.
Show resolved Hide resolved
);
Copy link
Contributor

Choose a reason for hiding this comment

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

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

const all: string[] = [
...new Set([...Object.keys(dependencies), ...Object.keys(peerDependencies)]),
]
Expand Down
5 changes: 2 additions & 3 deletions code/core/scripts/helpers/generatePackageJsonFile.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { writeFile } from 'node:fs/promises';
import { readFile, writeFile } from 'node:fs/promises';
import { join, relative } from 'node:path';

import { readJSON } from 'fs-extra';
import slash from 'slash';

import { sortPackageJson } from '../../../../scripts/prepare/tools';
Expand All @@ -11,7 +10,7 @@ const cwd = process.cwd();

export async function generatePackageJsonFile(entries: ReturnType<typeof getEntries>) {
const location = join(cwd, 'package.json');
const pkgJson = await readJSON(location);
const pkgJson = JSON.parse(await readFile(location, { encoding: 'utf-8' }));

/**
* Re-create the `exports` field in `code/core/package.json` This way we only need to update the
Expand Down
7 changes: 4 additions & 3 deletions code/core/scripts/helpers/generateTypesMapperFiles.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { closeSync, existsSync, openSync } from 'node:fs';
import { writeFile } from 'node:fs/promises';
import { join, relative } from 'node:path';

import { ensureFile } from 'fs-extra';

import { dedent } from '../../../../scripts/prepare/tools';
import type { getEntries } from '../entries';

Expand Down Expand Up @@ -34,7 +33,9 @@ export async function generateTypesMapperFiles(entries: ReturnType<typeof getEnt
await Promise.all(
all.map(async (filePath) => {
const location = filePath.replace('src', 'dist').replace(/\.tsx?/, '.d.ts');
await ensureFile(location);
if (!existsSync(location)) {
JReinhold marked this conversation as resolved.
Show resolved Hide resolved
closeSync(openSync(location, 'w'));
ziebam marked this conversation as resolved.
Show resolved Hide resolved
}
await writeFile(location, await generateTypesMapperContent(filePath));
})
);
Expand Down
8 changes: 4 additions & 4 deletions code/core/scripts/prep.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
/* eslint-disable local-rules/no-uncategorized-errors */
import { watch } from 'node:fs';
import { existsSync, mkdirSync, watch } from 'node:fs';
import { mkdir, rm, writeFile } from 'node:fs/promises';
import { dirname, join } from 'node:path';

import { ensureDir } from 'fs-extra';

import {
chalk,
dedent,
Expand Down Expand Up @@ -320,7 +318,9 @@ async function run() {
const outName =
keys.length === 1 ? dirname(keys[0]).replace('dist/', '') : `meta-${format}-${index}`;

await ensureDir('report');
if (!existsSync('report')) {
mkdirSync('report');
}
await writeFile(`report/${outName}.json`, JSON.stringify(out.metafile, null, 2));
await writeFile(
`report/${outName}.txt`,
Expand Down
15 changes: 7 additions & 8 deletions code/core/src/builder-manager/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { cp, rm, writeFile } from 'node:fs/promises';
import { dirname, join, parse } from 'node:path';

import { stringifyProcessEnvs } from '@storybook/core/common';
Expand All @@ -9,7 +10,6 @@ import { globalExternals } from '@fal-works/esbuild-plugin-global-externals';
import { pnpPlugin } from '@yarnpkg/esbuild-plugin-pnp';
import aliasPlugin from 'esbuild-plugin-alias';
import express from 'express';
import fs from 'fs-extra';

import type {
BuilderBuildResult,
Expand Down Expand Up @@ -149,7 +149,7 @@ const starter: StarterFunction = async function* starterGeneratorFn({
// make sure we clear output directory of addons dir before starting
// this could cause caching issues where addons are loaded when they shouldn't
const addonsDir = config.outdir;
await fs.remove(addonsDir);
await rm(addonsDir, { recursive: true, force: true });

yield;

Expand Down Expand Up @@ -256,14 +256,17 @@ const builder: BuilderFunction = async function* builderGeneratorFn({ startTime,

yield;

const managerFiles = fs.copy(coreDirOrigin, coreDirTarget, {
// TODO: `fsPromises.cp` is marked as experimental in Node 16-21. Ask in the PR whether we should
// use it anyway or stick to `fs-extra` for now.
const managerFiles = cp(coreDirOrigin, coreDirTarget, {
filter: (src) => {
const { ext } = parse(src);
if (ext) {
return ext === '.js';
}
return true;
},
recursive: true,
});
const { cssFiles, jsFiles } = await readOrderedFiles(addonsDir, compilation?.outputFiles);

Expand All @@ -288,11 +291,7 @@ const builder: BuilderFunction = async function* builderGeneratorFn({ startTime,
globals
);

await Promise.all([
//
fs.writeFile(join(options.outputDir, 'index.html'), html),
managerFiles,
]);
await Promise.all([writeFile(join(options.outputDir, 'index.html'), html), managerFiles]);

logger.trace({ message: '=> Manager built', time: process.hrtime(startTime) });

Expand Down
Loading