From 8e0ef7fcfda98563ae4ee12478bc8a57d50c88cc Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Fri, 29 Nov 2024 12:09:15 +1100 Subject: [PATCH 1/4] Fix two issues with anonymous id calculation: - Paths were not stable between windows + unix - Remote URLs can be specified without the `.git` extension --- code/core/src/telemetry/anonymous-id.test.ts | 16 ++++++++++++++- code/core/src/telemetry/anonymous-id.ts | 21 +++++++++++++------- code/deprecated/telemetry/package.json | 3 +++ 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/code/core/src/telemetry/anonymous-id.test.ts b/code/core/src/telemetry/anonymous-id.test.ts index 6606f43cabc2..5968528f32ba 100644 --- a/code/core/src/telemetry/anonymous-id.test.ts +++ b/code/core/src/telemetry/anonymous-id.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { normalizeGitUrl } from './anonymous-id'; +import { normalizeGitUrl, unhashedProjectId } from './anonymous-id'; describe('normalizeGitUrl', () => { it('trims off https://', () => { @@ -69,6 +69,12 @@ describe('normalizeGitUrl', () => { ); }); + it('adds .git if missing', () => { + expect(normalizeGitUrl('https://github.com/storybookjs/storybook')).toEqual( + 'github.com/storybookjs/storybook.git' + ); + }); + it('trims off #hash', () => { expect(normalizeGitUrl('https://github.com/storybookjs/storybook.git#next')).toEqual( 'github.com/storybookjs/storybook.git' @@ -85,3 +91,11 @@ describe('normalizeGitUrl', () => { ); }); }); + +describe('unhashedProjectId', () => { + it('normalizes windows paths', () => { + expect( + unhashedProjectId('https://github.com/storybookjs/storybook.git\n', 'path\\to\\storybook') + ).toBe('github.com/storybookjs/storybook.gitpath/to/storybook'); + }); +}); diff --git a/code/core/src/telemetry/anonymous-id.ts b/code/core/src/telemetry/anonymous-id.ts index c97071c0d127..1d23d0a493ae 100644 --- a/code/core/src/telemetry/anonymous-id.ts +++ b/code/core/src/telemetry/anonymous-id.ts @@ -3,6 +3,7 @@ import { relative } from 'node:path'; import { getProjectRoot } from '@storybook/core/common'; import { execSync } from 'child_process'; +import slash from 'slash'; import { oneWayHash } from './one-way-hash'; @@ -16,7 +17,18 @@ export function normalizeGitUrl(rawUrl: string) { // Now strip off scheme const urlWithoutScheme = urlWithoutUser.replace(/^.*\/\//, ''); - return urlWithoutScheme.replace(':', '/'); + // Ensure the URL ends in `.git` + const urlWithExtension = urlWithoutScheme.endsWith('.git') + ? urlWithoutScheme + : `${urlWithoutScheme}.git`; + + return urlWithExtension.replace(':', '/'); +} + +// we use a combination of remoteUrl and working directory +// to separate multiple storybooks from the same project (e.g. monorepo) +export function unhashedProjectId(remoteUrl: string, projectRootPath: string) { + return `${normalizeGitUrl(remoteUrl)}${slash(projectRootPath)}`; } let anonymousProjectId: string; @@ -25,7 +37,6 @@ export const getAnonymousProjectId = () => { return anonymousProjectId; } - let unhashedProjectId; try { const projectRoot = getProjectRoot(); @@ -36,11 +47,7 @@ export const getAnonymousProjectId = () => { stdio: `pipe`, }); - // we use a combination of remoteUrl and working directory - // to separate multiple storybooks from the same project (e.g. monorepo) - unhashedProjectId = `${normalizeGitUrl(String(originBuffer))}${projectRootPath}`; - - anonymousProjectId = oneWayHash(unhashedProjectId); + anonymousProjectId = oneWayHash(unhashedProjectId(String(originBuffer), projectRootPath)); } catch (_) { // } diff --git a/code/deprecated/telemetry/package.json b/code/deprecated/telemetry/package.json index 211104ad6fae..aaf4b347c384 100644 --- a/code/deprecated/telemetry/package.json +++ b/code/deprecated/telemetry/package.json @@ -38,6 +38,9 @@ "*.cjs", "*.d.ts" ], + "devDependencies": { + "slash": "^5.0.0" + }, "peerDependencies": { "storybook": "^8.2.0 || ^8.3.0-0 || ^8.4.0-0 || ^8.5.0-0 || ^8.6.0-0" }, From 37d9355fa68b918c524ded8a81167a759f5a3376 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Fri, 29 Nov 2024 12:11:13 +1100 Subject: [PATCH 2/4] Add a unix test --- code/core/src/telemetry/anonymous-id.test.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/code/core/src/telemetry/anonymous-id.test.ts b/code/core/src/telemetry/anonymous-id.test.ts index 5968528f32ba..d583fc75096f 100644 --- a/code/core/src/telemetry/anonymous-id.test.ts +++ b/code/core/src/telemetry/anonymous-id.test.ts @@ -93,6 +93,13 @@ describe('normalizeGitUrl', () => { }); describe('unhashedProjectId', () => { + it('does not touch unix paths', () => { + expect( + unhashedProjectId('https://github.com/storybookjs/storybook.git\n', 'path/to/storybook') + ).toBe('github.com/storybookjs/storybook.gitpath/to/storybook'); + }); + + it('normalizes windows paths', () => { expect( unhashedProjectId('https://github.com/storybookjs/storybook.git\n', 'path\\to\\storybook') From 8e36f89d555d8f6d3ae37f69bc2b2c94539da046 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Fri, 29 Nov 2024 12:23:38 +1100 Subject: [PATCH 3/4] Didn't need to add the dependency to the `telemetry` package --- code/deprecated/telemetry/package.json | 3 --- 1 file changed, 3 deletions(-) diff --git a/code/deprecated/telemetry/package.json b/code/deprecated/telemetry/package.json index aaf4b347c384..211104ad6fae 100644 --- a/code/deprecated/telemetry/package.json +++ b/code/deprecated/telemetry/package.json @@ -38,9 +38,6 @@ "*.cjs", "*.d.ts" ], - "devDependencies": { - "slash": "^5.0.0" - }, "peerDependencies": { "storybook": "^8.2.0 || ^8.3.0-0 || ^8.4.0-0 || ^8.5.0-0 || ^8.6.0-0" }, From 1cc488e7e065bb02017a781112d2f6cda00f44ab Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Fri, 29 Nov 2024 12:53:24 +1100 Subject: [PATCH 4/4] Minor linting issue --- code/core/src/telemetry/anonymous-id.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/code/core/src/telemetry/anonymous-id.test.ts b/code/core/src/telemetry/anonymous-id.test.ts index d583fc75096f..8277ca547e85 100644 --- a/code/core/src/telemetry/anonymous-id.test.ts +++ b/code/core/src/telemetry/anonymous-id.test.ts @@ -93,13 +93,12 @@ describe('normalizeGitUrl', () => { }); describe('unhashedProjectId', () => { - it('does not touch unix paths', () => { + it('does not touch unix paths', () => { expect( unhashedProjectId('https://github.com/storybookjs/storybook.git\n', 'path/to/storybook') ).toBe('github.com/storybookjs/storybook.gitpath/to/storybook'); }); - it('normalizes windows paths', () => { expect( unhashedProjectId('https://github.com/storybookjs/storybook.git\n', 'path\\to\\storybook')