Sandboxes: Fix generation #32334

ndelangen · 2025-08-27T11:58:58Z

What I did

The sandbox generation script had been failing for a long time.

I'm making it work again, by addressing multiple underlaying issues.

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

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

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

Greptile Summary

This PR fixes sandbox generation issues in the Storybook project by addressing problems with local registry handling and Yarn 1 compatibility. The main changes include:

Sandbox Generation Script Refactoring: The scripts/sandbox/generate.ts file has been significantly refactored to improve local registry handling. The withLocalRegistry function was simplified by removing its dependency on the package manager and hardcoding the registry URL restoration. A new addResolutions function was introduced that directly modifies package.json files instead of using the package manager API for adding resolutions. The main generation process is now wrapped in a withLocalRegistry context to ensure all sandbox operations occur within the proper registry environment.

Import Path Optimization: The code/core/src/shared/status-store/index.ts file was updated to use a more local import path for the StoryId type, changing from 'storybook/internal/csf' to '../../types'. This improves the internal module structure while maintaining the same functionality since the types module re-exports the same type.

Yarn 1 CI Compatibility: The Yarn1Proxy.ts file was modified to conditionally include the --ignore-workspace-root-check flag based on the environment. In CI environments, this flag is omitted, while in local development it's included. This addresses installation failures that were occurring in CI environments during sandbox generation.

These changes work together to resolve sandbox generation failures that were likely caused by package manager API limitations, registry configuration issues, and environment-specific Yarn 1 behavior. The refactoring moves away from relying on package manager abstractions toward more direct file system operations for better reliability across different environments.

PR Description Notes:

The "What I did" section is empty and should be filled with a description of the changes
No testing approach is specified in the manual testing section
No documentation updates are indicated despite the internal changes

Confidence score: 4/5

This PR addresses specific technical issues with sandbox generation and appears well-targeted to fix the underlying problems
Score reflects solid technical approach but some complexity in the registry handling changes that could have edge cases
Pay close attention to the scripts/sandbox/generate.ts file due to significant refactoring of registry and package resolution logic

…x script - Changed import of StoryId to use the correct path in status-store. - Modified CI detection logic to include both GITHUB_ACTIONS and CI environment variables. - Simplified local registry handling in the sandbox generation script. - Added console logs for better debugging during the Storybook initialization process.

- Updated Yarn1Proxy to conditionally set install arguments based on CI environment. - Added functionality to add resolutions in the sandbox generation script for better dependency management. - Improved error handling and logging during sandbox generation process.

greptile-apps

_{3 files reviewed, no comments}

_{Edit Code Review Bot Settings | Greptile}

- Changed cron syntax to use single quotes for consistency. - Updated branch references in the workflow to target 'norbert/fix-sandbox-generation'. - Adjusted failure notification message to reflect the correct branch context. - Commented out the 'generate-main' job to prevent execution during this workflow.

nx-cloud · 2025-08-27T12:05:12Z

View your CI Pipeline Execution ↗ for commit e2da611

Command	Status	Duration	Result
`nx run-many -t build --parallel=3`	✅ Succeeded	1m 11s	View ↗

☁️ Nx Cloud last updated this comment at 2025-08-27 15:25:28 UTC

- Updated the readFile function to specify 'utf-8' encoding when reading package.json. - Changed the JSON parsing method to use JSON.parse for better clarity and consistency.

- Modified the SOLID generator to include an option to disable component addition. - Removed commented-out resolution for 'jackspeak' from the sandbox generation script to streamline the code.

ndelangen · 2025-08-27T13:22:44Z

code/lib/create-storybook/src/generators/SOLID/index.ts

@@ -9,7 +9,7 @@ const generator: Generator = async (packageManager, npmOptions, options) => {
    npmOptions,
    { ...options, builder: CoreBuilder.Vite },
    'solid',
-    {},
+    { addComponents: false },


This likely sneaked under the radar when the sandbox generation was broken, the code adding components, via static copy, hard fails due to renderer not supported

ndelangen · 2025-08-27T13:23:19Z

code/core/src/shared/status-store/index.ts

@@ -1,8 +1,7 @@
-import type { StoryId } from 'storybook/internal/csf';


I experienced a race-condition compiling, as StoryId is simply a string, there really is no meaningful difference here.

ndelangen · 2025-08-27T13:25:04Z

code/core/src/common/js-package-manager/Yarn1Proxy.ts

@@ -36,7 +36,7 @@ export class Yarn1Proxy extends JsPackageManager {

  getInstallArgs(): string[] {
    if (!this.installArgs) {
-      this.installArgs = ['--ignore-workspace-root-check'];


This one is puzzling... but this flag seemed to not be allowed sometimes, even though it's yarn 1.

I do not have an explanation for why... but I do know the flag is never useful for sandbox we generate, because those are never generated in a monorepo, but always in a temp directory, in isolation.

ndelangen · 2025-08-27T13:27:18Z

scripts/sandbox/generate.ts

-      await withLocalRegistry({
-        packageManager,
-        action: async () => {
-          await packageManager.addPackageResolutions({
-            ...storybookVersions,
-            // Yarn1 Issue: https://github.com/storybookjs/storybook/issues/22431
-            jackspeak: '2.1.1',
-          });
-
-          await sbInit(tmpDir, env, [...flags, '--package-manager=yarn1'], debug);
-        },
-        cwd: tmpDir,
-        env,
-        debug,
-      });


Instead of wrapping every sandbox generation with a withLocalRegistry, I wrapped the whole job in it, greatly simplifying and reducing the amount of noise in the job.

ndelangen · 2025-08-27T13:29:07Z

scripts/sandbox/generate.ts

+async function addResolutions(beforeDir: string) {
+  const packageJson = await readFile(join(beforeDir, 'package.json'), 'utf-8').then((c) =>
+    JSON.parse(c)
+  );
+
+  packageJson.resolutions = {
+    ...storybookVersions,
+  };
+
+  await writeFile(join(beforeDir, 'package.json'), JSON.stringify(packageJson, null, 2));
+}


I do this manually, because using the JSPackageManager-class, just freezes up the whole CI.

I suspect it's something related/due to the upgrade to jiti@2, but I don't know how I'd prove this without spending a lot of time.

ndelangen · 2025-08-27T13:31:06Z

scripts/sandbox/generate.ts

@@ -104,27 +99,13 @@ const addStorybook = async ({
  try {
    await copy(beforeDir, tmpDir);

-    const packageManager = JsPackageManagerFactory.getPackageManager({ force: 'yarn1' }, tmpDir);


This was the line, the CI got stuck on.

Since it's used in a very limited way, it's not adding a lot to actually use it, so by replacing this with a direct in-file implementation the CI was no longer blocked.

- Added a mock for the 'node:process' module in Yarn1Proxy.test.ts to set the CI environment variable to 'false'. - Updated Yarn1Proxy.ts to import the 'node:process' module for improved environment handling.

ndelangen · 2025-08-27T13:37:24Z

scripts/sandbox/generate.ts

-          await packageManager.addPackageResolutions({
-            ...storybookVersions,
-            // Yarn1 Issue: https://github.com/storybookjs/storybook/issues/22431
-            jackspeak: '2.1.1',


no longer needed AFAICS

- Modified the mock for the 'node:process' module in Yarn1Proxy.test.ts to explicitly set the CI environment variable to 'false' for improved test reliability.

- Updated the mock for the 'node:process' module in Yarn1Proxy.test.ts to set the CI environment variable to a boolean value instead of a string, enhancing consistency in test behavior.

yannbf · 2025-08-27T14:42:34Z

scripts/sandbox/generate.ts

  } catch (e) {
+    console.log('error', e);


is this intentional?

yannbf · 2025-08-27T14:45:26Z

scripts/sandbox/generate.ts

@@ -397,7 +399,15 @@ if (esMain(import.meta.url)) {
    .option('--debug', 'Print all the logs to the console')
    .option('--local-registry', 'Use local registry', false)
    .action((optionValues) => {
-      generate(optionValues)
+      withLocalRegistry({


This should only be called if --local-registry flag is passed

- Updated the generate.ts script to check for the --local-registry option and execute the appropriate action based on its value, improving flexibility in the sandbox generation process.

ndelangen added 2 commits August 27, 2025 12:30

greptile-apps bot reviewed Aug 27, 2025

View reviewed changes

ndelangen added build Internal-facing build tooling & test updates ci:normal labels Aug 27, 2025

ndelangen self-assigned this Aug 27, 2025

ndelangen added 3 commits August 27, 2025 14:13

Fix package.json reading in sandbox generation script

74982c0

- Updated the readFile function to specify 'utf-8' encoding when reading package.json. - Changed the JSON parsing method to use JSON.parse for better clarity and consistency.

Update SOLID generator options and clean up sandbox generation script

c564837

- Modified the SOLID generator to include an option to disable component addition. - Removed commented-out resolution for 'jackspeak' from the sandbox generation script to streamline the code.

reset

1636f57

ndelangen commented Aug 27, 2025

View reviewed changes

Enhance Yarn1Proxy tests with process environment mocking

7586294

- Added a mock for the 'node:process' module in Yarn1Proxy.test.ts to set the CI environment variable to 'false'. - Updated Yarn1Proxy.ts to import the 'node:process' module for improved environment handling.

ndelangen commented Aug 27, 2025

View reviewed changes

ndelangen added 3 commits August 27, 2025 15:38

Merge branch 'next' into norbert/fix-sandbox-generation

606e51a

Update Yarn1Proxy tests to set CI environment variable to 'false'

650a252

- Modified the mock for the 'node:process' module in Yarn1Proxy.test.ts to explicitly set the CI environment variable to 'false' for improved test reliability.

Refactor Yarn1Proxy test environment mocking

815b48d

- Updated the mock for the 'node:process' module in Yarn1Proxy.test.ts to set the CI environment variable to a boolean value instead of a string, enhancing consistency in test behavior.

yannbf reviewed Aug 27, 2025

View reviewed changes

scripts/sandbox/generate.ts

} catch (e) {

console.log('error', e);

Copy link

Member

yannbf Aug 27, 2025

Choose a reason for hiding this comment

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

is this intentional?

yannbf reviewed Aug 27, 2025

View reviewed changes

yannbf approved these changes Aug 27, 2025

View reviewed changes

Refactor sandbox generation script to conditionally use local registry

e2da611

- Updated the generate.ts script to check for the --local-registry option and execute the appropriate action based on its value, improving flexibility in the sandbox generation process.

ndelangen merged commit 197ef41 into next Aug 27, 2025
54 of 55 checks passed

ndelangen deleted the norbert/fix-sandbox-generation branch August 27, 2025 15:25

github-actions bot mentioned this pull request Sep 2, 2025

Release: Prerelease 10.0.0-beta.1 #32305

Merged

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Sandboxes: Fix generation #32334

Sandboxes: Fix generation #32334

Uh oh!

ndelangen commented Aug 27, 2025 •

edited

Loading

Uh oh!

greptile-apps bot left a comment

Uh oh!

nx-cloud bot commented Aug 27, 2025 •

edited

Loading

Uh oh!

ndelangen Aug 27, 2025

Uh oh!

ndelangen Aug 27, 2025

Uh oh!

ndelangen Aug 27, 2025

Uh oh!

ndelangen Aug 27, 2025

Uh oh!

ndelangen Aug 27, 2025

Uh oh!

ndelangen Aug 27, 2025

Uh oh!

ndelangen Aug 27, 2025

Uh oh!

yannbf Aug 27, 2025

Uh oh!

yannbf Aug 27, 2025

Uh oh!

Uh oh!

Uh oh!

		@@ -1,8 +1,7 @@
		import type { StoryId } from 'storybook/internal/csf';

Uh oh!

Sandboxes: Fix generation #32334

Sandboxes: Fix generation #32334

Uh oh!

Conversation

ndelangen commented Aug 27, 2025 • edited 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

Confidence score: 4/5

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

nx-cloud bot commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ndelangen commented Aug 27, 2025 •

edited

Loading

nx-cloud bot commented Aug 27, 2025 •

edited

Loading