Replace: Use empathic over find-up#31338
Conversation
There was a problem hiding this comment.
50 file(s) reviewed, 25 comment(s)
Edit PR Review Bot Settings | Greptile
There was a problem hiding this comment.
50 file(s) reviewed, 8 comment(s)
Edit PR Review Bot Settings | Greptile
|
View your CI Pipeline Execution ↗ for commit 6ff320a
☁️ Nx Cloud last updated this comment at |
|
I ran the package benchmark script locally, and this is the comparison with Raw data output{
"@storybook/addon-vitest": {
"package": "@storybook/addon-vitest",
"dependencyCount": { "base": 6, "new": 6, "diff": 0 },
"selfSize": { "base": 617673, "new": 607478, "diff": -10195 },
"dependencySize": { "base": 1491763, "new": 1491763, "diff": 0 }
},
"@storybook/builder-vite": {
"package": "@storybook/builder-vite",
"dependencyCount": { "base": 6, "new": 6, "diff": 0 },
"selfSize": { "base": 447736, "new": 379690, "diff": -68046 },
"dependencySize": { "base": 832164, "new": 832164, "diff": 0 }
},
"storybook": {
"package": "storybook",
"dependencyCount": { "base": 49, "new": 49, "diff": 0 },
"selfSize": { "base": 31768394, "new": 31731776, "diff": -36618 },
"dependencySize": { "base": 16954299, "new": 16954299, "diff": 0 }
},
"@storybook/angular": {
"package": "@storybook/angular",
"dependencyCount": { "base": 250, "new": 245, "diff": -5 },
"selfSize": { "base": 607199, "new": 611242, "diff": 4043 },
"dependencySize": { "base": 30771133, "new": 30742945, "diff": -28188 }
},
"@storybook/ember": {
"package": "@storybook/ember",
"dependencyCount": { "base": 250, "new": 245, "diff": -5 },
"selfSize": { "base": 28901, "new": 30254, "diff": 1353 },
"dependencySize": { "base": 30008812, "new": 29980603, "diff": -28209 }
},
"@storybook/html-vite": {
"package": "@storybook/html-vite",
"dependencyCount": { "base": 9, "new": 9, "diff": 0 },
"selfSize": { "base": 23349, "new": 23349, "diff": 0 },
"dependencySize": { "base": 1323445, "new": 1255399, "diff": -68046 }
},
"@storybook/nextjs": {
"package": "@storybook/nextjs",
"dependencyCount": { "base": 535, "new": 531, "diff": -4 },
"selfSize": { "base": 215398, "new": 215398, "diff": 0 },
"dependencySize": { "base": 58596539, "new": 58572458, "diff": -24081 }
},
"@storybook/nextjs-vite": {
"package": "@storybook/nextjs-vite",
"dependencyCount": { "base": 129, "new": 124, "diff": -5 },
"selfSize": { "base": 2390330, "new": 2390330, "diff": 0 },
"dependencySize": { "base": 22401141, "new": 22304870, "diff": -96271 }
},
"@storybook/preact-vite": {
"package": "@storybook/preact-vite",
"dependencyCount": { "base": 9, "new": 9, "diff": 0 },
"selfSize": { "base": 12873, "new": 12873, "diff": 0 },
"dependencySize": { "base": 1308696, "new": 1240650, "diff": -68046 }
},
"@storybook/react-native-web-vite": {
"package": "@storybook/react-native-web-vite",
"dependencyCount": { "base": 162, "new": 157, "diff": -5 },
"selfSize": { "base": 34896, "new": 34767, "diff": -129 },
"dependencySize": { "base": 23535145, "new": 23438874, "diff": -96271 }
},
"@storybook/react-vite": {
"package": "@storybook/react-vite",
"dependencyCount": { "base": 122, "new": 117, "diff": -5 },
"selfSize": { "base": 31538, "new": 31501, "diff": -37 },
"dependencySize": { "base": 20504966, "new": 20408732, "diff": -96234 }
},
"@storybook/react-webpack5": {
"package": "@storybook/react-webpack5",
"dependencyCount": { "base": 323, "new": 319, "diff": -4 },
"selfSize": { "base": 24572, "new": 24572, "diff": 0 },
"dependencySize": { "base": 44607291, "new": 44583236, "diff": -24055 }
},
"@storybook/svelte-vite": {
"package": "@storybook/svelte-vite",
"dependencyCount": { "base": 18, "new": 18, "diff": 0 },
"selfSize": { "base": 53169, "new": 53169, "diff": 0 },
"dependencySize": { "base": 25918079, "new": 25850033, "diff": -68046 }
},
"@storybook/sveltekit": {
"package": "@storybook/sveltekit",
"dependencyCount": { "base": 19, "new": 19, "diff": 0 },
"selfSize": { "base": 50541, "new": 50541, "diff": 0 },
"dependencySize": { "base": 25972050, "new": 25904004, "diff": -68046 }
},
"@storybook/vue3-vite": {
"package": "@storybook/vue3-vite",
"dependencyCount": { "base": 104, "new": 104, "diff": 0 },
"selfSize": { "base": 34316, "new": 34194, "diff": -122 },
"dependencySize": { "base": 42678151, "new": 42620343, "diff": -57808 }
},
"@storybook/web-components-vite": {
"package": "@storybook/web-components-vite",
"dependencyCount": { "base": 10, "new": 10, "diff": 0 },
"selfSize": { "base": 19918, "new": 19918, "diff": 0 },
"dependencySize": { "base": 1355743, "new": 1287697, "diff": -68046 }
},
"sb": {
"package": "sb",
"dependencyCount": { "base": 50, "new": 50, "diff": 0 },
"selfSize": { "base": 1216, "new": 1216, "diff": 0 },
"dependencySize": { "base": 48723194, "new": 48686576, "diff": -36618 }
},
"@storybook/cli": {
"package": "@storybook/cli",
"dependencyCount": { "base": 324, "new": 324, "diff": 0 },
"selfSize": { "base": 238917, "new": 238917, "diff": 0 },
"dependencySize": { "base": 97046716, "new": 96992691, "diff": -54025 }
},
"@storybook/codemod": {
"package": "@storybook/codemod",
"dependencyCount": { "base": 267, "new": 267, "diff": 0 },
"selfSize": { "base": 30919, "new": 30919, "diff": 0 },
"dependencySize": { "base": 81481310, "new": 81444692, "diff": -36618 }
},
"create-storybook": {
"package": "create-storybook",
"dependencyCount": { "base": 1, "new": 1, "diff": 0 },
"selfSize": { "base": 10988718, "new": 10971311, "diff": -17407 },
"dependencySize": { "base": 97745, "new": 97745, "diff": 0 }
},
"@storybook/preset-react-webpack": {
"package": "@storybook/preset-react-webpack",
"dependencyCount": { "base": 175, "new": 171, "diff": -4 },
"selfSize": { "base": 23715, "new": 23671, "diff": -44 },
"dependencySize": { "base": 30142179, "new": 30118168, "diff": -24011 }
}
}This is an AI summary based on the data: @storybook/addon-vitest
@storybook/builder-vite
storybook
@storybook/angular
@storybook/ember
@storybook/html-vite
@storybook/nextjs
@storybook/nextjs-vite
@storybook/preact-vite
@storybook/react-native-web-vite
@storybook/react-vite
@storybook/react-webpack5
@storybook/svelte-vite
@storybook/sveltekit
@storybook/vue3-vite
@storybook/web-components-vite
sb
@storybook/cli
@storybook/codemod
create-storybook
@storybook/preset-react-webpack
Summary of Most Impactful Changes
The most impactful change overall appears to be the optimization of the |
|
i've read through all of it and it looks good other than a few minor changes If the team don't want to land this as one big change, I think we should split it up per monorepo package (core, presets, frameworks, etc) if we do that, I don't think we need a PR per dependency change. just one per monorepo package |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 18 | 18 | 0 |
| Self size | 2.01 MB | 2.01 MB | 0 B |
| Dependency size | 9.40 MB | 9.43 MB | 🚨 +28 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-vitest
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 6 | 6 | 0 |
| Self size | 495 KB | 489 KB | 🎉 -5 KB 🎉 |
| Dependency size | 1.49 MB | 1.52 MB | 🚨 +28 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@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 | 🚨 +18 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/builder-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 187 | 187 | 0 |
| Self size | 68 KB | 68 KB | 🚨 +36 B 🚨 |
| Dependency size | 31.67 MB | 31.68 MB | 🚨 +10 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 48 | 48 | 0 |
| Self size | 30.42 MB | 30.49 MB | 🚨 +74 KB 🚨 |
| Dependency size | 17.61 MB | 17.64 MB | 🚨 +28 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 187 | 187 | 0 |
| Self size | 134 KB | 126 KB | 🎉 -8 KB 🎉 |
| Dependency size | 29.89 MB | 29.90 MB | 🚨 +10 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 192 | 191 | 🎉 -1 🎉 |
| Self size | 17 KB | 17 KB | 🎉 -43 B 🎉 |
| Dependency size | 28.39 MB | 28.40 MB | 🚨 +11 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/html-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 14 | 14 | 0 |
| Self size | 23 KB | 23 KB | 🚨 +2 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 | 🚨 +2 B 🚨 |
| Dependency size | 21.66 MB | 21.56 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 | 🚨 +19 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 | 🚨 +2 B 🚨 |
| Dependency size | 23.04 MB | 22.95 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/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 199 | 199 | 0 |
| Self size | 17 KB | 17 KB | 0 B |
| Dependency size | 32.92 MB | 32.93 MB | 🚨 +10 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 22 | 22 | 0 |
| Self size | 59 KB | 59 KB | 🚨 +18 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 | 🎉 -30 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 | 🎉 -75 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 | 0 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 | 876 KB | 🎉 -10 KB 🎉 |
| Dependency size | 81.67 MB | 81.78 MB | 🚨 +107 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 174 | 174 | 0 |
| Self size | 35 KB | 35 KB | 0 B |
| Dependency size | 76.74 MB | 76.85 MB | 🚨 +112 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.03 MB | 48.13 MB | 🚨 +102 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.83 MB | 30.84 MB | 🚨 +10 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
|
I think everything passes now? :) |
|
@beeequeue @43081j I think it's probably fine as one big change (don't quote me!) but the biggest issue is that we're all hands on deck for our 9.0 release and are starting to lock down. If we don't get it into 9.0 we'll def get it into 9.1 though! |
|
Just want to also say thank you for prioritizing the cleanup of storybook as much as you have! It was one of my biggest gripes with the tool back in the day - and while we can't use it at work yet because of them not prioritizing cleanup work - the benefits to the ecosystem cannot be understated ❤️ |
|
Thanks so much for the kind words @beeequeue. It's PRs like this that make it happen, and the team is so grateful for your help getting things into shape. We've made some big strides but there's still a lot more we can do!!! 🩷 |
|
@beeequeue Do you think you could have a look at this PR and maybe update it with the changes from the base-branch? 🙏 |
|
the sandbox tests are failing again 🫠 |
|
@43081j @ndelangen @shilman I think everything has passed now except for a single test that timed out, can you try re-running that job to see if it's just flaky? |
|
O no more conflicts! Possibly an unwelcome distraction, but I did notice a few users of |
|
@ndelangen this is all looking mighty green now except for the flaky unit test? |
empathic where possibleempathic over find-up
Closes #29103
What I did
This PR replaces a number of packages (see #29103) with
empathic, a smaller, faster alternative.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
There is one part I am the most unsure about, which is the removed/changed
safeResolves.I'm not entirely sure how to test it properly or exactly where they're used, so I would love some pointers for how to handle those.
Documentation
MIGRATION.MD
in that one file 😄
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake 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/coreteam 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>Greptile Summary
This PR replaces multiple utility packages with 'empathic' across the Storybook codebase, aiming to improve performance and reduce bundle size by consolidating path resolution and package finding functionality.
find-up,find-cache-dir, andresolve-fromwith equivalentempathicfunctions in core packages and frameworkssafeResolve.tsutility in favor ofempathic/resolve, requiring careful testing of module resolution behaviorempathic/find, particularly in package manager proxiesempathic's APIempathicv1.1.0💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!