Skip to content

CLI: Fix automigration bugs found in real monorepos - #36476

Closed
valentinpalkovic wants to merge 22 commits into
nextfrom
fix/automigrate-main-mts-and-dirname
Closed

valentinpalkovic wants to merge 22 commits into
nextfrom
fix/automigrate-main-mts-and-dirname

Conversation

@valentinpalkovic

@valentinpalkovic valentinpalkovic commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Closes #

What I did

Running storybook upgrade from a local registry against ten open-source projects on Storybook 10 (calcite-design-system, vue-dsfr, pmndrs/docs, podman-desktop, eui, grommet, twenty, gutenberg, spartan, dxos) turned up these bugs on next.
Every one of them reproduced with and without #36462, so they are fixed here rather than there.
Each one is small, but together they make an upgrade slow, incomplete, or broken in exactly the projects with many Storybooks.

Bug Where it showed up Effect
vitest-setup-file check reads every .gitignore in the repo, once per project dxos, 130 Storybooks about 18 of the 21 seconds of automigration detection
findConfigFile does not know main.mts / main.cts dxos, 127 of 131 configs are main.mts automigrations that edit main silently skip those projects
wrap-getAbsolutePath imports dirname next to an existing const dirname twenty (twenty-ui, twenty-front-component-renderer) the migrated main.ts no longer parses
upgrade finds .storybook directories inside node_modules twenty (4 templates in @nx/storybook) 7 projects detected instead of 3, and 4 errors
storybook add progress lines print inside the automigration task log, with an absolute path twenty (addon-mcp) the list of ✔ and ✕ results is broken up by long wrapped lines
pnpm install -w after the upgrade podman-desktop (pnpm 12) the lockfile keeps every workspace package on Storybook 10; build fails on the frozen lockfile
a pnpm catalog: dependency is upgraded in package.json calcite catalog: references are replaced by pinned versions, the catalog stays on 10.3.6
satellite addons take their next tag podman-desktop @storybook/addon-svelte-csf goes from 5.1.2 down to 5.0.0-next.30
upgrade-storybook-related-dependencies crosses a major inside a package family spartan @nx/storybook 22 → 23 while every other @nx/* package stays on 22
core.builder wrapped by wrap-getAbsolutePath is imported as a directory calcite storybook build fails with ERR_UNSUPPORTED_DIR_IMPORT
upgrade --yes asks for a config directory when there is no .storybook grommet, gutenberg (storybook/) the unattended run hangs on a prompt
automigrate --yes runs opt-in fixes, upgrade --yes silently leaves them out spartan, pmndrs/docs a framework switch runs unasked, or a detected fix never runs and is never named
storybook add writes recast's output, eslintPlugin reprints the whole config, imports go above a license header grommet, gutenberg, spartan, eui one-line edits restyle whole files; eui's copyright header is no longer first
task logs redraw every message, config parse warnings and eslint notices print per project dxos, 131 Storybooks 3.6 MB of terminal output, mostly repeated lines
a colored task log line close to the terminal width is miscounted by clack twenty the list printed above the task log is erased
addon-mcp fails on a main config that storybook add cannot edit, detection only counts fixes that apply to every project, a failed fix shows no reason dxos, gutenberg, twenty 129 of 130 dxos projects end with a failed migration, "No automigrations detected" is printed before three fixes run, vitest-setup-file's manual steps only reach the debug log
vitest-setup-file removes the first import together with its leading comment dxos the setup file loses its copyright header

1. findFilesUp scans every .gitignore for a top-level file lookup

findFilesUp looks for vitest.config.* and vite.config.* in each directory from the project up to the root, with globby({ gitignore: true }).
The patterns only match files directly in each directory, but gitignore: true makes globby read every .gitignore below it first.
In a monorepo the vitest-setup-file check runs this from the workspace root for every project.

-    matchingFiles.push(
-      ...globbySync(matchers, {
-        gitignore: true,
-        absolute: true,
-        cwd: directory,
-      })
-    );
+    matchingFiles.push(...globbySync(matchers, { absolute: true, cwd: directory }));

Detection plus apply of the automigrations on dxos (131 Storybooks, of which 130 load: 131 main.*, 127 preview.*, 3 manager.*, and 1,310 story files), median of 3 runs, before and after this PR:

Time File reads Most reads of one file
next 21.3 s 5,858 321 (packages/plugins/plugin-github/.gitignore)
this PR 1.8 s 557 132

The CPU profile of the next run attributes 18.1 s of the 18.2 s detection to findFilesUp inside the vitest-setup-file check.

2. findConfigFile misses .mts and .cts configs

findConfigFile kept its own extension list (ts, js, tsx, jsx, mjs, cjs), while Storybook loads configs with the shared storybookConfigExtensions, which also includes .mts, .cts, .mtsx, and .ctsx.
For a main.mts project getStorybookData returns no mainConfigPath, so every automigration that edits main treats the project as having none.
findConfigFile now uses the shared list, which also makes its lookup order match the loader's.

3. wrap-getAbsolutePath writes a duplicate dirname

The fix always imported dirname from node:path, even when the main config declares its own.
This is what next writes for twenty-ui, captured from a real run:

-import path from 'node:path';
+import path, { dirname } from 'node:path';
 import { fileURLToPath } from 'node:url';
 ...
 const dirname =
   typeof __dirname !== 'undefined'

The fix now imports under an alias when the name is taken, and the generated helper calls that alias:

import { dirname as pathDirname } from 'node:path';
import path from 'node:path';
import { fileURLToPath } from 'node:url';

const dirname = path.dirname(fileURLToPath(import.meta.url));
// ...
function getAbsolutePath(value) {
  return pathDirname(fileURLToPath(import.meta.resolve(`${value}/package.json`)));
}

After this PR, all three migrated main.ts files in twenty parse again.

4. Project discovery relies on .gitignore to skip node_modules

findStorybookProjects globs for **/.storybook with gitignore: true and nothing else.
twenty ignores node_modules with the pattern **/**/node_modules/, which git honors but globby does not, so upgrade detected the .storybook templates that @nx/storybook ships:

◇  7 projects detected
│  ✔ /packages/twenty-front/.storybook
│  ✔ /packages/twenty-front-component-renderer/.storybook
│  ✔ /packages/twenty-ui/.storybook
│  There were some errors while collecting data for the following projects:
│  ✕ /node_modules/@nx/storybook/src/generators/configuration/files/v10/project-files/.storybook
│  ...

Discovery now always ignores **/node_modules/**. upgrade --dry-run on twenty now reports 3 projects detected.

5. storybook add output inside the automigration log

addon-mcp and angular-to-angular-vite call add(), which logged Verifying, Installing, and Adding '@storybook/addon-mcp' to the "addons" field in /absolute/path/main.ts between the per-fix results.
Those automigrations now pass a logger that sends these lines to debug, so the result list stays a clean column of ✔ and ✕.
A standalone storybook add prints the same lines as before, with the main config path now relative to the working directory.

6. pnpm 12 installs only the workspace root

The upgrade ran pnpm install -w at a workspace root.
pnpm 12 treats that as an install of the root project only, so no other workspace project is re-resolved.
A two-package workspace shows the difference when the child's dependency changes from 6.0.0 to 7.0.0:

pnpm 10.18.0 install -w -> lock: specifier: 7.0.0
pnpm 12.4.2 install -w -> lock: specifier: 6.0.0

install now runs without -w; pnpm add keeps it, since pnpm requires it there.
On podman-desktop the upgrade now logs Scope: all 26 workspace projects, and storybook build --test passes the frozen-lockfile check.

7. Dependency upgrades that went the wrong way

  • A dependency declared through a pnpm catalog now has its catalog entry upgraded in pnpm-workspace.yaml, keeping the entry's range modifier and the file's comments, and keeps its catalog: reference.
  • A satellite addon takes the newer of its latest and next tags, so podman-desktop's @storybook/addon-svelte-csf goes to 5.1.4.
  • A scoped package whose scope has installed siblings that are not upgraded stays within its major, so spartan's @nx/storybook goes to 22.7.12 instead of 23.

8. A builder given as its package directory

wrap-getAbsolutePath wraps core.builder, so the builder reaches Storybook as …/node_modules/@storybook/builder-vite, and ES modules cannot import a directory.
Storybook now resolves a package directory to its entry point before importing it.

9. --yes runs that need a human, or skip fixes without saying so

  • With no .storybook directory, upgrade --yes now stops the spinner and exits:

    ■  No .storybook directory was found. Pass --config-dir <path> to upgrade a Storybook whose configuration lives elsewhere.
    
  • upgrade --yes now names the opt-in fixes it leaves out:

    Not run with --yes because they are opt-in (run `storybook automigrate <id>` to apply one):
    
  • storybook automigrate --yes applies the same rule and runs an opt-in fix, such as a framework switch, only when the user names it.

10. Edits that restyled whole files

  • storybook add wrote recast's output as is, which collapsed grommet's multi-line addons array into one line and indented spartan's tab-indented main.ts with 8 spaces. It now formats its edit with the Prettier the project installed and the project's Prettier config, and leaves it as printed when the project has none. It does not use the shared formatFileContent, whose fallback to Prettier's defaults for an .editorconfig would restyle a whole file that was only edited.
  • The eslint plugin setup inserts its import and spread as text instead of reprinting the file with recast. grommet's eslint.config.mjs changes by 3 lines instead of 219.
  • New imports go below the existing ones when the first statement carries a leading comment, so eui's license header stays first. vitest-setup-file keeps that comment when it removes the first import.

11. Terminal output in large monorepos

On dxos (131 Storybooks: 131 main.*, 127 preview.*, 3 manager.*, 702 story files) the upgrade printed 3,606,836 bytes.
Four changes bring it to 220,913 bytes:

Change Before
a running task log shows its latest 10 lines and keeps the rest for an error 10,440 "Checking automigrations for"
config export shapes that cannot be parsed statically log at debug level, with the file 647 "CSF Parsing error"
the eslint plugin check stays quiet when a project has no eslint config 10,337 "Unable to find eslint"
detection counts a fix that applies to any project, not only to all of them "No automigrations detected", then three fixes ran

A task log line whose color codes would push it onto an extra terminal row loses its colors, because clack counts rows by the raw length and erased the lines printed above the log.
A failed fix now shows its message in the project's task log, so vitest-setup-file's manual steps reach the terminal.
addon-mcp skips a main config that storybook add cannot edit instead of failing on 129 of dxos' 130 projects.

Checklist for Contributors

Testing

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

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

Caution

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

  1. Clone https://github.com/twentyhq/twenty and install dependencies (yarn install --mode=skip-build).
  2. Run npx storybook@<canary> automigrate wrap-getAbsolutePath --config-dir packages/twenty-ui/.storybook --yes.
    Confirm packages/twenty-ui/.storybook/main.ts imports { dirname as pathDirname } from 'node:path' and that npx storybook dev --config-dir packages/twenty-ui/.storybook still starts.
  3. In a project whose config is .storybook/main.mts, run npx storybook@<canary> automigrate --yes and confirm migrations that edit main (for example enable-experimental-review --yes) now update main.mts.
  4. In a monorepo with several Storybooks, run npx storybook@<canary> upgrade --yes and confirm automigration detection no longer stalls on the vitest-setup-file check.
  5. In twenty, run npx storybook@<canary> upgrade --dry-run and confirm it reports 3 projects detected, with no paths inside node_modules.
  6. In twenty, run npx storybook@<canary> upgrade --yes and confirm each project's list shows only ✔ and ✕ lines, without Verifying or Adding ... to the "addons" field lines.
  7. Clone https://github.com/grommet/grommet (config in storybook/), install, and run npx storybook@<canary> upgrade --yes.
    Confirm it exits right away with the Pass --config-dir <path> hint instead of waiting for input.
    Run it again with --config-dir storybook and confirm eslint.config.mjs gains only the plugin import and one spread, and that storybook/main.js keeps its multi-line addons array.
  8. Clone https://github.com/podman-desktop/podman-desktop (pnpm 12), install, and run npx storybook@<canary> upgrade --yes.
    Confirm pnpm-lock.yaml now resolves every workspace package to the new Storybook version, and that @storybook/addon-svelte-csf is not downgraded to a next prerelease.
  9. Clone https://github.com/Esri/calcite-design-system (pnpm catalog), install, and run npx storybook@<canary> upgrade --yes.
    Confirm the Storybook entries of the catalog in pnpm-workspace.yaml are upgraded and package.json keeps its catalog: references.
  10. In a project with a failing automigration, such as twenty's vitest-setup-file, confirm the terminal shows the fix's manual steps under its ✕ line.

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-storybook/src/sandbox-templates.ts

  • Declare whether manual QA will be needed for this PR during the next release, through qa:needed or qa:skip

  • 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.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor
Fails
🚫

PR is not labeled with one of: ["cleanup","BREAKING CHANGE","feature request","bug","documentation","maintenance","build","dependencies"]

🚫

PR is not labeled with one of: ["ci:normal","ci:merged","ci:daily","ci:docs"]

🚫

PR is not labeled with one of: ["qa:needed","qa:skip","qa:success"]

Generated by 🚫 dangerJS against 1fad805

@valentinpalkovic valentinpalkovic changed the title CLI: Fix three automigration bugs found in real monorepos CLI: Fix automigration bugs found in real monorepos Sep 28, 2026
- findConfigFile now finds main.mts, main.cts and the other config
  extensions Storybook loads, so automigrations no longer skip those
  projects.
- findFilesUp no longer reads every .gitignore below the project root on
  each call; the vitest-setup-file check took about 18 seconds in a
  monorepo with 130 Storybooks.
- wrap-getAbsolutePath renames its dirname import when the main config
  already declares dirname, instead of writing a duplicate declaration.
findStorybookProjects relied on .gitignore to skip node_modules, but globby
misses patterns like **/**/node_modules/. In twenty, upgrade detected four
.storybook templates from @nx/storybook as projects and reported errors
for them.
…orybook add

storybook add printed the absolute path of the main config, which wraps
across several lines in an upgrade of a monorepo.
…k log

addon-mcp and angular-to-angular-vite call add(), whose Verifying,
Installing and Adding lines broke up the list of fix results. They now
pass a logger that sends log lines to debug; storybook add itself prints
the same as before.
pnpm 12 treats `pnpm install -w` as an install of the workspace root
project only, so the upgrade left every other workspace project on its old
Storybook version in the lockfile. Keep `-w` for `pnpm add`, where pnpm
requires it at the workspace root.
With --yes, the upgrade printed "Running all detected automigrations" but
silently left out the opt-in ones it had just counted as detected. List them
separately with how to apply one.
… dir

When no .storybook directory is found, upgrade asked for its path even with
--yes, and did so while the detection spinner was still running, so an
unattended run hung. Stop the spinner before prompting, and with --yes fail
with a pointer to --config-dir.
setupAddonInConfig wrote recast's output as is, so adding an addon printed
the new addons entry with recast's default indentation, for example 8 spaces
in a tab-indented file. Run the project's formatter before writing, as the
automigrations do.
ConfigFile prepended every new import, so a main config that starts with a
license comment got the imports above it (seen in elastic/eui). When the
first statement has a leading comment, add new imports below the existing
ones, or take the comment over when the file has no imports.
configureFlatConfig reprinted the whole config with recast, so adding the
plugin re-indented every object in grommet's eslint.config.mjs (219 changed
lines) and wrote a double-quoted import into a single-quoted file. Insert the
import after the existing imports and the spread after the last array or
argument item as text, in the file's quote style and layout, and run the
project's formatter on the result.
Clack redraws a whole task log on every update, so a log taller than the
terminal repeats in the scrollback: the automigration detection over dxos'
130 Storybooks rewrote each line about 125 times, and its package.json list
showed twice. Show the latest ten lines by default and retain the rest, so a
log shown on error is still complete.
…config

The check logged a warning for every such Storybook, 130 times in dxos, for a
fix that simply does not apply there.
Upgrading to a prerelease took each satellite addon's `next` version, which
for @storybook/addon-svelte-csf is 5.0.0-next.30 while `latest` is 5.1.4, so
podman-desktop went from 5.1.2 down to a prerelease. Take whichever of the two
tags is newer.
… dependencies

upgrade-storybook-related-dependencies moved @nx/storybook from 22 to 23
while every other @nx package stayed on 22 (spartan), leaving unmet peers and
a version that still does not support Storybook 11. A package whose scope has
installed siblings that are not upgraded now stays within its major.
…cies

Upgrading a project whose Storybook packages come from a pnpm catalog
replaced every `catalog:` reference in package.json with a pinned version and
left the catalog in pnpm-workspace.yaml on the old version (calcite). Update
the catalog entry, keeping its range modifier and the file's comments, and
leave the `catalog:` reference alone.
…the file

Parsing a config whose default export is not an object literal, such as
`export default createConfig()`, printed a warning without saying which file.
The upgrade over dxos' 131 Storybooks printed about 1,250 such lines. A later
mutation of that config already fails with its own diagnostic, so log the
parse notice at debug level and name the file.
wrap-getAbsolutePath wraps core.builder, so the builder reaches Storybook as
a directory such as .../@storybook/builder-vite, and importing a directory
fails in ES modules (calcite's build: ERR_UNSUPPORTED_DIR_IMPORT). Resolve a
package directory to its entry point before importing it.
… import

Removing setProjectAnnotations also removes imports that become unused, and
when that was the file's first statement its leading comment went with it
(dxos' tools/storybook-react/.storybook/vitest.setup.ts lost its copyright
header). Move that comment onto the new first statement.
The optional addon-mcp migration failed on every main config that spreads a
shared config or exports a factory call: 129 of dxos' 130 Storybooks and one
of gutenberg's two ended with a failed migration. Try the edit in memory
during the check and skip such configs.
`storybook automigrate --yes` accepted every detected fix, including opt-in
framework switches such as nextjs-to-nextjs-vite, while `upgrade --yes`
leaves them out. Apply the same rule: with --yes, an opt-in fix runs only
when the user named it, as in `storybook automigrate nextjs-to-nextjs-vite
--yes`.
storybook add's edit of vue-dsfr's main.ts (no Prettier config, only an
.editorconfig) rewrote 79 lines with Prettier's default quotes and
semicolons, and gutenberg's lost wp-prettier's paren spacing on 66 lines.
Clack counts a task log line's terminal rows by its length including color
codes and erases that many rows on the next update. Colored lines close to
the terminal width therefore erased lines printed before the log: with the
manual steps of twenty's failing vitest-setup-file shown in the task log, the
list of opt-in automigrations above it vanished. Drop the colors of a line
only when they push it onto an extra row.
@valentinpalkovic

Copy link
Copy Markdown
Contributor Author

Split into one PR per fix, so each can be reviewed and merged on its own:

PR Fix
#36538 CLI: Find main.mts and main.cts configs in automigrations
#36539 CLI: Stop reading every .gitignore when looking for a Vite config
#36540 CLI: Rename the dirname import when main config declares its own
#36541 CLI: Never look for Storybook projects inside node_modules
#36542 CLI: Keep storybook add's progress lines out of the automigration log
#36543 CLI: Install every pnpm workspace project after an upgrade
#36544 CLI: Treat opt-in automigrations the same with automigrate --yes and upgrade --yes
#36545 CLI: Fail upgrade --yes with a hint instead of prompting for a config dir
#36546 CLI: Keep main and eslint config edits to the lines they change
#36547 CLI: Keep a license header first when editing config and setup files
#36548 Node logger: Keep task logs short and from erasing the output above them
#36549 CLI: Log per-project config notices at debug level
#36550 CLI: Never downgrade a satellite addon to its next tag
#36551 CLI: Keep scoped package families on one major when upgrading related dependencies
#36552 CLI: Upgrade pnpm catalog entries instead of pinning catalog dependencies
#36553 Core: Load a builder that main config gives as its package directory
#36554 CLI: Skip addon-mcp for a main config that storybook add cannot edit

Two fixes from this PR are dropped because they already landed on next in 7150c40: counting an automigration that applies to only some projects as detected, and showing a failed fix's message in the project's task log.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant