Skip to content

Commit

Permalink
Merge pull request #28965 from tobiasdiez/sandbox-improv
Browse files Browse the repository at this point in the history
Chore: Improve varous aspects of sandbox creation
  • Loading branch information
valentinpalkovic authored Nov 1, 2024
2 parents fb79a2b + ee5a4f6 commit db6bb01
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 19 deletions.
25 changes: 13 additions & 12 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Thank you for contributing to Storybook! Please submit all PRs to the `next` bra
<!-- Please check (put an "x" inside the "[ ]") the applicable items below to communicate how to test your changes -->

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

- [ ] stories
- [ ] unit tests
- [ ] integration tests
Expand Down Expand Up @@ -46,21 +47,21 @@ _This section is mandatory for all contributions. If you believe no manual test

## 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/src/sandbox-templates.ts`
- [ ] 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:
<details>
<summary>Available labels</summary>

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

</details>

### 🦋 Canary release
Expand All @@ -74,4 +75,4 @@ _core team members can create a canary release [here](https://github.com/storybo
<!-- CANARY_RELEASE_SECTION -->

<!-- BENCHMARK_SECTION -->
<!-- BENCHMARK_SECTION -->
<!-- BENCHMARK_SECTION -->
2 changes: 1 addition & 1 deletion docs/contribute/code.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ npx storybook@next link --local /path/to/local-repro-directory

## Developing a template

The first step is to add an entry to `code/lib/cli/src/sandbox-templates.ts`, which is the master list of all repro templates:
The first step is to add an entry to `code/lib/cli-storybook/src/sandbox-templates.ts`, which is the master list of all repro templates:

```ts
'cra/default-js': {
Expand Down
19 changes: 16 additions & 3 deletions scripts/sandbox/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { Options as ExecaOptions } from 'execa';
// eslint-disable-next-line depend/ban-dependencies
import { execaCommand } from 'execa';
// eslint-disable-next-line depend/ban-dependencies
import { copy, emptyDir, ensureDir, move, remove, rename, writeFile } from 'fs-extra';
import { copy, emptyDir, ensureDir, move, remove, writeFile } from 'fs-extra';
import pLimit from 'p-limit';
import { join, relative } from 'path';
import prettyTime from 'pretty-hrtime';
Expand Down Expand Up @@ -129,7 +129,8 @@ const addStorybook = async ({
throw e;
}

await rename(tmpDir, afterDir);
await copy(tmpDir, afterDir);
await remove(tmpDir);
};

export const runCommand = async (script: string, options: ExecaOptions, debug = false) => {
Expand Down Expand Up @@ -197,7 +198,19 @@ const runGenerators = async (
// We do the creation inside a temp dir to avoid yarn container problems
const createBaseDir = await temporaryDirectory();
if (!script.includes('pnp')) {
await setupYarn({ cwd: createBaseDir });
try {
await setupYarn({ cwd: createBaseDir });
} catch (error) {
const message = `❌ Failed to setup yarn in template: ${name} (${dirName})`;
if (isCI) {
ghActions.error(dedent`${message}
${(error as any).stack}`);
} else {
console.error(message);
console.error(error);
}
throw new Error(message);
}
}

const createBeforeDir = join(createBaseDir, BEFORE_DIR_NAME);
Expand Down
5 changes: 3 additions & 2 deletions scripts/sandbox/utils/yarn.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import fs from 'fs';
// eslint-disable-next-line depend/ban-dependencies
import { move, remove } from 'fs-extra';
import { join } from 'path';
Expand All @@ -12,7 +13,7 @@ interface SetupYarnOptions {

export async function setupYarn({ cwd, pnp = false, version = 'classic' }: SetupYarnOptions) {
// force yarn
await runCommand(`touch yarn.lock`, { cwd });
fs.writeFileSync(join(cwd, 'yarn.lock'), '', { flag: 'a' });
await runCommand(`yarn set version ${version}`, { cwd });
if (version === 'berry' && !pnp) {
await runCommand('yarn config set nodeLinker node-modules', { cwd });
Expand All @@ -22,7 +23,7 @@ export async function setupYarn({ cwd, pnp = false, version = 'classic' }: Setup

export async function localizeYarnConfigFiles(baseDir: string, beforeDir: string) {
await Promise.allSettled([
runCommand(`touch yarn.lock`, { cwd: beforeDir }),
fs.writeFileSync(join(beforeDir, 'yarn.lock'), '', { flag: 'a' }),
move(join(baseDir, '.yarn'), join(beforeDir, '.yarn')),
move(join(baseDir, '.yarnrc.yml'), join(beforeDir, '.yarnrc.yml')),
move(join(baseDir, '.yarnrc'), join(beforeDir, '.yarnrc')),
Expand Down
2 changes: 1 addition & 1 deletion scripts/utils/yarn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export const configureYarn2ForVerdaccio = async ({
// ⚠️ Need to set registry because Yarn 2 is not using the conf of Yarn 1 (URL is hardcoded in CircleCI config.yml)
`yarn config set npmRegistryServer "http://localhost:6001/"`,
// Some required magic to be able to fetch deps from local registry
`yarn config set unsafeHttpWhitelist --json '["localhost"]'`,
`yarn config set unsafeHttpWhitelist "localhost"`,
// Disable fallback mode to make sure everything is required correctly
`yarn config set pnpFallbackMode none`,
// We need to be able to update lockfile when bootstrapping the examples
Expand Down

0 comments on commit db6bb01

Please sign in to comment.