Skip to content

Commit a497d12

Browse files
alan-agius4dgp1130
authored andcommitted
refactor(@angular/cli): remove hasAnalyticsConfig analytics logic
With this change we clean up further the analytics code and re-use the `getAnalytics` to determine if the config is set or not. Also, this change inclused a refactor to the `createAnalytics` method to make it more readable.
1 parent 1a36fd9 commit a497d12

File tree

2 files changed

+47
-35
lines changed

2 files changed

+47
-35
lines changed

packages/angular/cli/bin/postinstall/analytics-prompt.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ try {
1717
var analytics = require('../../src/analytics/analytics');
1818

1919
analytics
20-
.hasAnalyticsConfig('global')
20+
.getAnalytics('global')
2121
.then((hasGlobalConfig) => {
2222
if (!hasGlobalConfig) {
2323
return analytics.promptAnalytics(true /** global */);

packages/angular/cli/src/analytics/analytics.ts

Lines changed: 46 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -152,16 +152,21 @@ export async function promptAnalytics(global: boolean, force = false): Promise<b
152152

153153
/**
154154
* Get the analytics object for the user.
155+
*
156+
* @returns
157+
* - `AnalyticsCollector` when enabled.
158+
* - `analytics.NoopAnalytics` when disabled.
159+
* - `undefined` when not configured.
155160
*/
156161
export async function getAnalytics(
157162
level: 'local' | 'global',
158-
): Promise<AnalyticsCollector | undefined> {
163+
): Promise<AnalyticsCollector | analytics.NoopAnalytics | undefined> {
159164
analyticsDebug('getAnalytics');
160165

161166
if (analyticsDisabled) {
162167
analyticsDebug('NG_CLI_ANALYTICS is false');
163168

164-
return undefined;
169+
return new analytics.NoopAnalytics();
165170
}
166171

167172
try {
@@ -170,7 +175,9 @@ export async function getAnalytics(
170175
workspace?.getCli()['analytics'];
171176
analyticsDebug('Workspace Analytics config found: %j', analyticsConfig);
172177

173-
if (!analyticsConfig) {
178+
if (analyticsConfig === false) {
179+
return new analytics.NoopAnalytics();
180+
} else if (analyticsConfig === undefined || analyticsConfig === null) {
174181
return undefined;
175182
} else {
176183
let uid: string | undefined = undefined;
@@ -231,33 +238,49 @@ export async function createAnalytics(
231238
workspace: boolean,
232239
skipPrompt = false,
233240
): Promise<analytics.Analytics> {
234-
let config: analytics.Analytics | undefined;
235-
const isDisabledGlobally = (await getWorkspace('global'))?.getCli()['analytics'] === false;
236-
// If in workspace and global analytics is enabled, defer to workspace level
237-
if (workspace && !isDisabledGlobally) {
238-
const skipAnalytics = skipPrompt || analyticsDisabled;
239-
// TODO: This should honor the `no-interactive` option.
240-
// It is currently not an `ng` option but rather only an option for specific commands.
241-
// The concept of `ng`-wide options are needed to cleanly handle this.
242-
if (!skipAnalytics && !(await hasAnalyticsConfig('local'))) {
243-
await promptAnalytics(false);
241+
// Global config takes precedence over local config only for the disabled check.
242+
// IE:
243+
// global: disabled & local: enabled = disabled
244+
// global: id: 123 & local: id: 456 = 456
245+
246+
// check global
247+
const globalConfig = await getAnalytics('global');
248+
if (globalConfig instanceof analytics.NoopAnalytics) {
249+
return globalConfig;
250+
}
251+
252+
let config = globalConfig;
253+
// Not disabled globally, check locally.
254+
if (workspace) {
255+
let localConfig = await getAnalytics('local');
256+
if (localConfig === undefined) {
257+
if (!skipPrompt) {
258+
// local is not unset, prompt user.
259+
260+
// TODO: This should honor the `no-interactive` option.
261+
// It is currently not an `ng` option but rather only an option for specific commands.
262+
// The concept of `ng`-wide options are needed to cleanly handle this.
263+
await promptAnalytics(false);
264+
localConfig = await getAnalytics('local');
265+
}
266+
}
267+
268+
if (localConfig instanceof analytics.NoopAnalytics) {
269+
return localConfig;
270+
} else if (localConfig) {
271+
// Favor local settings over global when defined.
272+
config = localConfig;
244273
}
245-
config = await getAnalytics('local');
246-
} else {
247-
config = await getAnalytics('global');
248274
}
249275

276+
// Get shared analytics
277+
// TODO: evalute if this should be completly removed.
250278
const maybeSharedAnalytics = await getSharedAnalytics();
251-
252279
if (config && maybeSharedAnalytics) {
253280
return new analytics.MultiAnalytics([config, maybeSharedAnalytics]);
254-
} else if (config) {
255-
return config;
256-
} else if (maybeSharedAnalytics) {
257-
return maybeSharedAnalytics;
258-
} else {
259-
return new analytics.NoopAnalytics();
260281
}
282+
283+
return config ?? maybeSharedAnalytics ?? new analytics.NoopAnalytics();
261284
}
262285

263286
function analyticsConfigValueToHumanFormat(value: unknown): 'enabled' | 'disabled' | 'not set' {
@@ -295,14 +318,3 @@ export async function getAnalyticsInfoString(): Promise<string> {
295318
` + '\n'
296319
);
297320
}
298-
299-
export async function hasAnalyticsConfig(level: 'local' | 'global'): Promise<boolean> {
300-
try {
301-
const workspace = await getWorkspace(level);
302-
if (workspace?.getCli()['analytics'] !== undefined) {
303-
return true;
304-
}
305-
} catch {}
306-
307-
return false;
308-
}

0 commit comments

Comments
 (0)