Skip to content

Replace: Use empathic over find-up#32472

Merged
ndelangen merged 1 commit into
nextfrom
revert-32470-revert-31338-empathic
Sep 16, 2025
Merged

Replace: Use empathic over find-up#32472
ndelangen merged 1 commit into
nextfrom
revert-32470-revert-31338-empathic

Conversation

@ndelangen
Copy link
Copy Markdown
Member

@ndelangen ndelangen commented Sep 16, 2025

Reverts #32470

Greptile Summary

Updated On: 2025-09-16 11:53:08 UTC

This PR reverts a previous revert (PR #32470) to re-introduce the empathic package across the Storybook codebase, consolidating multiple file system utility packages (find-up, find-cache-dir, resolve-from, fd-package-json, read-pkg-up) into a single unified package. The change affects critical infrastructure throughout Storybook including:

  • Package manager proxies (NPM, Yarn1/2, PNPM, BUN) - Updates file discovery logic for package.json resolution and PnP API detection
  • Framework presets (Angular, React, Ember, Vue3) - Changes TypeScript config and package.json discovery patterns
  • Build systems (Vite, Webpack builders) - Updates cache directory resolution and config file finding
  • CLI tools and utilities - Modifies framework detection, ESLint plugin discovery, and project initialization logic
  • Core infrastructure - Changes telemetry metadata collection, story indexing, and reference resolution

Key API changes include switching from asynchronous find-up operations to synchronous empathic operations, parameter name changes (e.g., stopAt becomes last), and consolidating separate package functionality into unified empathic modules (empathic/find, empathic/package, empathic/walk, empathic/resolve). The change aims to improve performance and reduce bundle size by eliminating multiple utility dependencies in favor of a single optimized package.

Confidence score: 1/5

  • This PR has critical implementation inconsistencies that will cause immediate runtime failures and test failures
  • Multiple files show incomplete or incorrect reversions with API mismatches between test mocks and implementations, missing await keywords on asynchronous operations, and contradictory changes that don't align with the stated PR intent
  • Files throughout the codebase require immediate attention including test files with wrong mocks, implementation files missing proper async/await patterns, and package.json files with dependency mismatches

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Sep 16, 2025

View your CI Pipeline Execution ↗ for commit a4b6af6

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 49s View ↗

☁️ Nx Cloud last updated this comment at 2025-09-16 11:05:02 UTC

@storybook-app-bot
Copy link
Copy Markdown

Package Benchmarks

Commit: a4b6af6, ran on 16 September 2025 at 10:50:52 UTC

The following packages have significant changes to their size or dependencies:

@storybook/builder-vite

Before After Difference
Dependency count 11 11 0
Self size 374 KB 319 KB 🎉 -55 KB 🎉
Dependency size 1.30 MB 1.30 MB 0 B
Bundle Size Analyzer Link Link

storybook

Before After Difference
Dependency count 48 48 0
Self size 30.42 MB 30.49 MB 🚨 +79 KB 🚨
Dependency size 17.64 MB 17.64 MB 0 B
Bundle Size Analyzer Link Link

@storybook/html-vite

Before After Difference
Dependency count 14 14 0
Self size 23 KB 23 KB 🎉 -18 B 🎉
Dependency size 1.71 MB 1.66 MB 🎉 -55 KB 🎉
Bundle Size Analyzer Link Link

@storybook/nextjs-vite

Before After Difference
Dependency count 130 124 🎉 -6 🎉
Self size 4.00 MB 4.00 MB 0 B
Dependency size 21.66 MB 21.57 MB 🎉 -91 KB 🎉
Bundle Size Analyzer Link Link

@storybook/preact-vite

Before After Difference
Dependency count 14 14 0
Self size 14 KB 14 KB 0 B
Dependency size 1.70 MB 1.64 MB 🎉 -55 KB 🎉
Bundle Size Analyzer Link Link

@storybook/react-native-web-vite

Before After Difference
Dependency count 163 157 🎉 -6 🎉
Self size 31 KB 31 KB 🚨 +24 B 🚨
Dependency size 23.05 MB 22.96 MB 🎉 -91 KB 🎉
Bundle Size Analyzer Link Link

@storybook/react-vite

Before After Difference
Dependency count 120 114 🎉 -6 🎉
Self size 36 KB 36 KB 🚨 +23 B 🚨
Dependency size 19.60 MB 19.51 MB 🎉 -91 KB 🎉
Bundle Size Analyzer Link Link

@storybook/svelte-vite

Before After Difference
Dependency count 22 22 0
Self size 59 KB 59 KB 0 B
Dependency size 26.95 MB 26.89 MB 🎉 -55 KB 🎉
Bundle Size Analyzer Link Link

@storybook/sveltekit

Before After Difference
Dependency count 23 23 0
Self size 49 KB 49 KB 0 B
Dependency size 27.01 MB 26.95 MB 🎉 -55 KB 🎉
Bundle Size Analyzer Link Link

@storybook/vue3-vite

Before After Difference
Dependency count 110 109 🎉 -1 🎉
Self size 38 KB 38 KB 🎉 -47 B 🎉
Dependency size 43.82 MB 43.76 MB 🎉 -64 KB 🎉
Bundle Size Analyzer Link Link

@storybook/web-components-vite

Before After Difference
Dependency count 15 15 0
Self size 20 KB 20 KB 🚨 +18 B 🚨
Dependency size 1.74 MB 1.68 MB 🎉 -55 KB 🎉
Bundle Size Analyzer Link Link

@storybook/cli

Before After Difference
Dependency count 205 205 0
Self size 886 KB 883 KB 🎉 -3 KB 🎉
Dependency size 81.72 MB 81.79 MB 🚨 +75 KB 🚨
Bundle Size Analyzer Link Link

@storybook/codemod

Before After Difference
Dependency count 174 174 0
Self size 35 KB 35 KB 🚨 +42 B 🚨
Dependency size 76.78 MB 76.86 MB 🚨 +79 KB 🚨
Bundle Size Analyzer Link Link

create-storybook

Before After Difference
Dependency count 49 49 0
Self size 1.52 MB 1.52 MB 🎉 -5 KB 🎉
Dependency size 48.06 MB 48.13 MB 🚨 +79 KB 🚨
Bundle Size Analyzer node node

@storybook/preset-react-webpack

Before After Difference
Dependency count 170 170 0
Self size 26 KB 21 KB 🎉 -6 KB 🎉
Dependency size 30.84 MB 30.84 MB 0 B
Bundle Size Analyzer Link Link

@ndelangen ndelangen merged commit 6d0bb54 into next Sep 16, 2025
59 of 63 checks passed
@ndelangen ndelangen deleted the revert-32470-revert-31338-empathic branch September 16, 2025 11:24
Copy link
Copy Markdown
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.

52 files reviewed, 28 comments

Edit Code Review Bot Settings | Greptile

cwd: process.cwd(),
stopAt: getProjectRoot(),
});
const tsconfigPath = find.up('tsconfig.json', { cwd: process.cwd(), last: getProjectRoot() });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

logic: Critical: This synchronous call will block the event loop. The original findUp() was async and required await, but find.up() is synchronous. This could cause performance issues in webpack builds.

cwd: options.configDir,
stopAt: getProjectRoot(),
});
const location = pkg.up({ cwd: options.configDir, last: getProjectRoot() });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

logic: Critical: pkg.up appears synchronous but findUp was async. Verify this doesn't block the event loop or cause type mismatches since the function remains async.


import boxen, { type Options } from 'boxen';
import { findUpMultipleSync } from 'find-up';
import * as walk from 'empathic/walk';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

logic: Verify 'empathic' is added to dependencies, not just devDependencies, since it's imported in production code

{ cwd: this.primaryPackageJson.operationDir, stopAt: getProjectRoot() }
);
const wantedPath = join('node_modules', packageName, 'package.json');
const packageJsonPath = find.up(wantedPath, { cwd: this.cwd, last: getProjectRoot() });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

logic: Critical: find.up() is likely async but called without await. This will cause runtime errors if it returns a Promise.

const mockContext: any = {
...liveContext,
findUp: async ([name]: string[]) => name,
empathic: { any: ([name]: string[]) => name },
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

logic: API mismatch: mock uses any method but implementation calls find.any() with different parameters (last vs stopAt)

Comment on lines +80 to +81
const wantedPath = join('node_modules', packageName, 'package.json');
const packageJsonPath = find.up(wantedPath, { cwd: this.cwd, last: getProjectRoot() });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

logic: find.up from empathic is synchronous but the method is async. Also, the parameter last is empathic-specific (find-up uses stopAt). This suggests the wrong direction of changes.

Comment on lines +27 to +28
import * as find from 'empathic/find';
import * as pkg from 'empathic/package';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

logic: Critical inconsistency: PR claims to revert FROM empathic TO traditional packages, but these imports use empathic instead of expected 'find-up' and 'fd-package-json'

const docTSConfig = find.up('tsconfig.doc.json', {
cwd: options.configDir,
stopAt: getProjectRoot(),
last: getProjectRoot(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

logic: API parameter mismatch: empathic uses 'last' while find-up uses 'stopAt' - this suggests wrong direction of conversion


export default {
packageJson: sync({ cwd: process.cwd() }).packageJson,
packageJson: JSON.parse(readFileSync(pkg.up({ cwd: process.cwd() }))),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

style: Using JSON.parse(readFileSync(...)) removes error handling that read-pkg-up provided. This could throw if the file doesn't exist or contains invalid JSON.

"@storybook/global": "^5.0.0",
"babel-loader": "9.1.3",
"find-up": "^7.0.0"
"empathic": "2.0.0"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

logic: This dependency change appears to be backwards - adding empathic instead of removing it. For a revert PR, this should be changing back to find-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant