Skip to content

Commit

Permalink
cleanup(misc): improve check for whether stats should be recorded (#2…
Browse files Browse the repository at this point in the history
…3234)

<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

## Current Behavior
<!-- This is the behavior we have today -->

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #
  • Loading branch information
leosvelperez authored May 10, 2024
1 parent 7cf09a6 commit fd71b6b
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 8 deletions.
22 changes: 18 additions & 4 deletions packages/create-nx-workspace/src/utils/nx/ab-testing.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { execSync } from 'node:child_process';
import { isCI } from '../ci/is-ci';
import { getPackageManagerCommand } from '../package-manager';

export const NxCloudChoices = ['yes', 'github', 'circleci', 'skip'];

Expand Down Expand Up @@ -93,11 +95,9 @@ export async function recordStat(opts: {
meta: string[];
}) {
try {
const major = Number(opts.nxVersion.split('.')[0]);
if (process.env.NX_VERBOSE_LOGGING === 'true') {
console.log(`Record stat. Major: ${major}`);
if (!shouldRecordStats()) {
return;
}
if (major < 10 || major > 19) return; // test version, skip it
const axios = require('axios');
await (axios['default'] ?? axios)
.create({
Expand All @@ -116,3 +116,17 @@ export async function recordStat(opts: {
}
}
}

function shouldRecordStats(): boolean {
const pmc = getPackageManagerCommand();
try {
const stdout = execSync(pmc.getRegistryUrl, { encoding: 'utf-8' });
const url = new URL(stdout.trim());

// don't record stats when testing locally
return url.hostname !== 'localhost';
} catch {
// fallback to true if we can't detect the registry
return true;
}
}
6 changes: 6 additions & 0 deletions packages/create-nx-workspace/src/utils/package-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export function getPackageManagerCommand(
exec: string;
preInstall?: string;
globalAdd: string;
getRegistryUrl: string;
} {
const pmVersion = getPackageManagerVersion(packageManager);
const [pmMajor, pmMinor] = pmVersion.split('.');
Expand All @@ -54,6 +55,9 @@ export function getPackageManagerCommand(
// using npx is necessary to avoid yarn classic manipulating the version detection when using berry
exec: useBerry ? 'npx' : 'yarn',
globalAdd: 'yarn global add',
getRegistryUrl: useBerry
? 'yarn config get npmRegistryServer'
: 'yarn config get registry',
};

case 'pnpm':
Expand All @@ -65,13 +69,15 @@ export function getPackageManagerCommand(
install: 'pnpm install --no-frozen-lockfile --silent --ignore-scripts',
exec: useExec ? 'pnpm exec' : 'pnpx',
globalAdd: 'pnpm add -g',
getRegistryUrl: 'pnpm config get registry',
};

case 'npm':
return {
install: 'npm install --silent --ignore-scripts',
exec: 'npx',
globalAdd: 'npm i -g',
getRegistryUrl: 'npm config get registry',
};
}
}
Expand Down
22 changes: 18 additions & 4 deletions packages/nx/src/utils/ab-testing.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { execSync } from 'node:child_process';
import { isCI } from './is-ci';
import { getPackageManagerCommand } from './package-manager';

export type MessageOptionKey = 'yes' | 'skip';

Expand Down Expand Up @@ -75,11 +77,9 @@ export async function recordStat(opts: {
meta: string;
}) {
try {
const major = Number(opts.nxVersion.split('.')[0]);
if (process.env.NX_VERBOSE_LOGGING === 'true') {
console.log(`Record stat. Major: ${major}`);
if (!shouldRecordStats()) {
return;
}
if (major < 10 || major > 19) return; // test version, skip it
const axios = require('axios');
await (axios['default'] ?? axios)
.create({
Expand All @@ -98,3 +98,17 @@ export async function recordStat(opts: {
}
}
}

function shouldRecordStats(): boolean {
const pmc = getPackageManagerCommand();
try {
const stdout = execSync(pmc.getRegistryUrl, { encoding: 'utf-8' });
const url = new URL(stdout.trim());

// don't record stats when testing locally
return url.hostname !== 'localhost';
} catch {
// fallback to true if we can't detect the registry
return true;
}
}
6 changes: 6 additions & 0 deletions packages/nx/src/utils/package-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export interface PackageManagerCommands {
dlx: string;
list: string;
run: (script: string, args: string) => string;
getRegistryUrl: string;
}

/**
Expand Down Expand Up @@ -101,6 +102,9 @@ export function getPackageManagerCommand(
dlx: useBerry ? 'yarn dlx' : 'yarn',
run: (script: string, args: string) => `yarn ${script} ${args}`,
list: useBerry ? 'yarn info --name-only' : 'yarn list',
getRegistryUrl: useBerry
? 'yarn config get npmRegistryServer'
: 'yarn config get registry',
};
},
pnpm: () => {
Expand All @@ -123,6 +127,7 @@ export function getPackageManagerCommand(
? `pnpm run ${script} -- ${args}`
: `pnpm run ${script} ${args}`,
list: 'pnpm ls --depth 100',
getRegistryUrl: 'pnpm config get registry',
};
},
npm: () => {
Expand All @@ -140,6 +145,7 @@ export function getPackageManagerCommand(
dlx: 'npx',
run: (script: string, args: string) => `npm run ${script} -- ${args}`,
list: 'npm ls',
getRegistryUrl: 'npm config get registry',
};
},
};
Expand Down

0 comments on commit fd71b6b

Please sign in to comment.