diff --git a/code/core/src/common/js-package-manager/JsPackageManager.ts b/code/core/src/common/js-package-manager/JsPackageManager.ts index 1000cc7bf174..5a18b572f248 100644 --- a/code/core/src/common/js-package-manager/JsPackageManager.ts +++ b/code/core/src/common/js-package-manager/JsPackageManager.ts @@ -64,22 +64,6 @@ export abstract class JsPackageManager { return packageJSON ? packageJSON.version ?? null : null; } - // NOTE: for some reason yarn prefers the npm registry in - // local development, so always use npm - async setRegistryURL(url: string) { - if (url) { - await this.executeCommand({ command: 'npm', args: ['config', 'set', 'registry', url] }); - } else { - await this.executeCommand({ command: 'npm', args: ['config', 'delete', 'registry'] }); - } - } - - async getRegistryURL() { - const res = await this.executeCommand({ command: 'npm', args: ['config', 'get', 'registry'] }); - const url = res.trim(); - return url === 'undefined' ? undefined : url; - } - constructor(options?: JsPackageManagerOptions) { this.cwd = options?.cwd || process.cwd(); } @@ -487,6 +471,8 @@ export abstract class JsPackageManager { ): // Use generic and conditional type to force `string[]` if fetchAllVersions is true and `string` if false Promise; + public abstract getRegistryURL(): Promise; + public abstract runPackageCommand( command: string, args: string[], diff --git a/code/core/src/common/js-package-manager/NPMProxy.test.ts b/code/core/src/common/js-package-manager/NPMProxy.test.ts index d1bb289112f1..9c88ea60af45 100644 --- a/code/core/src/common/js-package-manager/NPMProxy.test.ts +++ b/code/core/src/common/js-package-manager/NPMProxy.test.ts @@ -34,21 +34,6 @@ describe('NPM Proxy', () => { }); }); - describe('setRegistryUrl', () => { - it('should run `npm config set registry https://foo.bar`', async () => { - const executeCommandSpy = vi.spyOn(npmProxy, 'executeCommand').mockResolvedValueOnce(''); - - await npmProxy.setRegistryURL('https://foo.bar'); - - expect(executeCommandSpy).toHaveBeenCalledWith( - expect.objectContaining({ - command: 'npm', - args: ['config', 'set', 'registry', 'https://foo.bar'], - }) - ); - }); - }); - describe('installDependencies', () => { describe('npm6', () => { it('should run `npm install`', async () => { diff --git a/code/core/src/common/js-package-manager/NPMProxy.ts b/code/core/src/common/js-package-manager/NPMProxy.ts index ff77aedfa95a..d4d711915925 100644 --- a/code/core/src/common/js-package-manager/NPMProxy.ts +++ b/code/core/src/common/js-package-manager/NPMProxy.ts @@ -181,6 +181,17 @@ export class NPMProxy extends JsPackageManager { }); } + public async getRegistryURL() { + const res = await this.executeCommand({ + command: 'npm', + // "npm config" commands are not allowed in workspaces per default + // https://github.com/npm/cli/issues/6099#issuecomment-1847584792 + args: ['config', 'get', 'registry', '-ws=false', '-iwr'], + }); + const url = res.trim(); + return url === 'undefined' ? undefined : url; + } + protected async runAddDeps(dependencies: string[], installAsDevDependencies: boolean) { const { logStream, readLogFile, moveLogFile, removeLogFile } = await createLogStream(); let args = [...dependencies]; diff --git a/code/core/src/common/js-package-manager/PNPMProxy.test.ts b/code/core/src/common/js-package-manager/PNPMProxy.test.ts index 2430221cc2e8..cca2ca4ea364 100644 --- a/code/core/src/common/js-package-manager/PNPMProxy.test.ts +++ b/code/core/src/common/js-package-manager/PNPMProxy.test.ts @@ -24,21 +24,6 @@ describe('PNPM Proxy', () => { }); }); - describe('setRegistryUrl', () => { - it('should run `npm config set registry https://foo.bar`', async () => { - const executeCommandSpy = vi.spyOn(pnpmProxy, 'executeCommand').mockResolvedValueOnce(''); - - await pnpmProxy.setRegistryURL('https://foo.bar'); - - expect(executeCommandSpy).toHaveBeenCalledWith( - expect.objectContaining({ - command: 'npm', - args: ['config', 'set', 'registry', 'https://foo.bar'], - }) - ); - }); - }); - describe('installDependencies', () => { it('should run `pnpm install`', async () => { const executeCommandSpy = vi diff --git a/code/core/src/common/js-package-manager/PNPMProxy.ts b/code/core/src/common/js-package-manager/PNPMProxy.ts index 41c2858763c8..6d3f434c87c9 100644 --- a/code/core/src/common/js-package-manager/PNPMProxy.ts +++ b/code/core/src/common/js-package-manager/PNPMProxy.ts @@ -90,6 +90,15 @@ export class PNPMProxy extends JsPackageManager { }); } + public async getRegistryURL() { + const res = await this.executeCommand({ + command: 'pnpm', + args: ['config', 'get', 'registry'], + }); + const url = res.trim(); + return url === 'undefined' ? undefined : url; + } + async runPackageCommand(command: string, args: string[], cwd?: string): Promise { return this.executeCommand({ command: 'pnpm', diff --git a/code/core/src/common/js-package-manager/Yarn1Proxy.test.ts b/code/core/src/common/js-package-manager/Yarn1Proxy.test.ts index a26ce81efeb9..c20f496fed80 100644 --- a/code/core/src/common/js-package-manager/Yarn1Proxy.test.ts +++ b/code/core/src/common/js-package-manager/Yarn1Proxy.test.ts @@ -25,21 +25,6 @@ describe('Yarn 1 Proxy', () => { }); }); - describe('setRegistryUrl', () => { - it('should run `yarn config set npmRegistryServer https://foo.bar`', async () => { - const executeCommandSpy = vi.spyOn(yarn1Proxy, 'executeCommand').mockResolvedValueOnce(''); - - await yarn1Proxy.setRegistryURL('https://foo.bar'); - - expect(executeCommandSpy).toHaveBeenCalledWith( - expect.objectContaining({ - command: 'npm', - args: ['config', 'set', 'registry', 'https://foo.bar'], - }) - ); - }); - }); - describe('installDependencies', () => { it('should run `yarn`', async () => { const executeCommandSpy = vi.spyOn(yarn1Proxy, 'executeCommand').mockResolvedValueOnce(''); diff --git a/code/core/src/common/js-package-manager/Yarn1Proxy.ts b/code/core/src/common/js-package-manager/Yarn1Proxy.ts index b193d4db4f15..44a8f0ca3d45 100644 --- a/code/core/src/common/js-package-manager/Yarn1Proxy.ts +++ b/code/core/src/common/js-package-manager/Yarn1Proxy.ts @@ -83,6 +83,15 @@ export class Yarn1Proxy extends JsPackageManager { return JSON.parse(readFileSync(packageJsonPath, 'utf-8')) as Record; } + public async getRegistryURL() { + const res = await this.executeCommand({ + command: 'yarn', + args: ['config', 'get', 'registry'], + }); + const url = res.trim(); + return url === 'undefined' ? undefined : url; + } + public async findInstallations(pattern: string[], { depth = 99 }: { depth?: number } = {}) { const yarnArgs = ['list', '--pattern', pattern.map((p) => `"${p}"`).join(' '), '--json']; diff --git a/code/core/src/common/js-package-manager/Yarn2Proxy.test.ts b/code/core/src/common/js-package-manager/Yarn2Proxy.test.ts index 21656ff1ccd2..4e4441aedb4d 100644 --- a/code/core/src/common/js-package-manager/Yarn2Proxy.test.ts +++ b/code/core/src/common/js-package-manager/Yarn2Proxy.test.ts @@ -53,21 +53,6 @@ describe('Yarn 2 Proxy', () => { }); }); - describe('setRegistryUrl', () => { - it('should run `yarn config set npmRegistryServer https://foo.bar`', async () => { - const executeCommandSpy = vi.spyOn(yarn2Proxy, 'executeCommand').mockResolvedValueOnce(''); - - await yarn2Proxy.setRegistryURL('https://foo.bar'); - - expect(executeCommandSpy).toHaveBeenCalledWith( - expect.objectContaining({ - command: 'npm', - args: ['config', 'set', 'registry', 'https://foo.bar'], - }) - ); - }); - }); - describe('addDependencies', () => { it('with devDep it should run `yarn install -D @storybook/core`', async () => { const executeCommandSpy = vi.spyOn(yarn2Proxy, 'executeCommand').mockResolvedValueOnce(''); diff --git a/code/core/src/common/js-package-manager/Yarn2Proxy.ts b/code/core/src/common/js-package-manager/Yarn2Proxy.ts index 7917bc7e1ebd..317a56b40090 100644 --- a/code/core/src/common/js-package-manager/Yarn2Proxy.ts +++ b/code/core/src/common/js-package-manager/Yarn2Proxy.ts @@ -239,6 +239,15 @@ export class Yarn2Proxy extends JsPackageManager { await removeLogFile(); } + public async getRegistryURL() { + const res = await this.executeCommand({ + command: 'yarn', + args: ['config', 'get', 'npmRegistryServer'], + }); + const url = res.trim(); + return url === 'undefined' ? undefined : url; + } + protected async runRemoveDeps(dependencies: string[]) { const args = [...dependencies]; diff --git a/scripts/sandbox/generate.ts b/scripts/sandbox/generate.ts index 22c50716cc81..88871e6eec4d 100755 --- a/scripts/sandbox/generate.ts +++ b/scripts/sandbox/generate.ts @@ -45,18 +45,34 @@ const sbInit = async ( await runCommand(`${sbCliBinaryPath} init ${fullFlags.join(' ')}`, { cwd, env }, debug); }; -const withLocalRegistry = async (packageManager: JsPackageManager, action: () => Promise) => { +type LocalRegistryProps = { + packageManager: JsPackageManager; + action: () => Promise; + cwd: string; + env: Record; + debug: boolean; +}; + +const withLocalRegistry = async ({ + packageManager, + action, + cwd, + env, + debug, +}: LocalRegistryProps) => { const prevUrl = await packageManager.getRegistryURL(); let error; try { console.log(`📦 Configuring local registry: ${LOCAL_REGISTRY_URL}`); - packageManager.setRegistryURL(LOCAL_REGISTRY_URL); + // NOTE: for some reason yarn prefers the npm registry in + // local development, so always use npm + await runCommand(`npm config set registry ${LOCAL_REGISTRY_URL}`, { cwd, env }, debug); await action(); } catch (e) { error = e; } finally { console.log(`📦 Restoring registry: ${prevUrl}`); - await packageManager.setRegistryURL(prevUrl); + await runCommand(`npm config set registry ${prevUrl}`, { cwd, env }, debug); if (error) { throw error; @@ -88,14 +104,20 @@ const addStorybook = async ({ const packageManager = JsPackageManagerFactory.getPackageManager({ force: 'yarn1' }, tmpDir); if (localRegistry) { - await withLocalRegistry(packageManager, 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); + 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, }); } else { await sbInit(tmpDir, env, [...flags, '--package-manager=yarn1'], debug); @@ -159,7 +181,7 @@ const runGenerators = async ( const baseDir = join(REPROS_DIRECTORY, dirName); const beforeDir = join(baseDir, BEFORE_DIR_NAME); try { - let flags: string[] = []; + let flags: string[] = ['--no-dev']; if (expected.renderer === '@storybook/html') flags = ['--type html']; else if (expected.renderer === '@storybook/server') flags = ['--type server'];