Skip to content

Commit bbdebf4

Browse files
rwaskiewiczrwaskiewicz
authored
chore(config): propagate ValidatedConfig through telemetry directory (#3484)
this commit updates `telemetry.ts#hasAppTarget` to use `ValidatedConfig` as its argument, rather than an unvalidated one. the single caller of this function (outside of tests) already has been converted to receive a `ValidatedConfig`, making this conversion a matter of only updating the function signature. tests for this function were updated to adhere to the stricter requirements of a `ValidatedConfig` this commit updates `telemetry.ts#getActiveTargets` to use `ValidatedConfig` as its argument, rather than an unvalidated one. the single caller of this function already has been converted to receive a `ValidatedConfig`, making this conversion a matter of only updating the function signature. this commit updates the aforementioned function to use `ValidatedConfig` as its argument, rather than an unvalidated one. the single caller of this function (outside of tests) already has been converted to receive a `ValidatedConfig`, making this conversion a matter of only updating the function signature. tests for this function were updated to adhere to the stricter requirements of a `ValidatedConfig`. because we now use a stricter object, it has additional fields on it. this broke the existing assertions on the function's tests. the tests have been updated to maintain the original spirit/intent of the assertions. `d.Config` was kept as the return type (and consequently the type in `CONFIG_PROPS_TO_DELETE`), as we're modifying/removing fields from the `ValidatedConfig`. this "felt" like it would inevitably throw compiler errors as we make `ValidatedConfig` stricter. if/when we decide to get rid of `d.Config`, we can revisit (and the compiler will make us!)
1 parent 8e03f45 commit bbdebf4

File tree

2 files changed

+72
-38
lines changed

2 files changed

+72
-38
lines changed

‎src/cli/telemetry/telemetry.ts‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,16 @@ export async function telemetryAction(
8282
}
8383
}
8484

85-
export function hasAppTarget(config: d.Config): boolean {
85+
/**
86+
* Helper function to determine if a Stencil configuration builds an application.
87+
*
88+
* This function is a rough approximation whether an application is generated as a part of a Stencil build, based on
89+
* contents of the project's `stencil.config.ts` file.
90+
*
91+
* @param config the configuration used by the Stencil project
92+
* @returns true if we believe the project generates an application, false otherwise
93+
*/
94+
export function hasAppTarget(config: d.ValidatedConfig): boolean {
8695
return config.outputTargets.some(
8796
(target) => target.type === WWW && (!!target.serviceWorker || (!!target.baseUrl && target.baseUrl !== '/'))
8897
);
@@ -92,7 +101,15 @@ export function isUsingYarn(sys: d.CompilerSystem) {
92101
return sys.getEnvironmentVar('npm_execpath')?.includes('yarn') || false;
93102
}
94103

95-
export async function getActiveTargets(config: d.Config): Promise<string[]> {
104+
/**
105+
* Build a list of the different types of output targets used in a Stencil configuration.
106+
*
107+
* Duplicate entries will not be returned from the list
108+
*
109+
* @param config the configuration used by the Stencil project
110+
* @returns a unique list of output target types found in the Stencil configuration
111+
*/
112+
export function getActiveTargets(config: d.ValidatedConfig): string[] {
96113
const result = config.outputTargets.map((t) => t.type);
97114
return Array.from(new Set(result));
98115
}
@@ -116,7 +133,7 @@ export const prepareData = async (
116133
): Promise<d.TrackableData> => {
117134
const { typescript, rollup } = coreCompiler.versions || { typescript: 'unknown', rollup: 'unknown' };
118135
const { packages, packagesNoVersions } = await getInstalledPackages(sys, config);
119-
const targets = await getActiveTargets(config);
136+
const targets = getActiveTargets(config);
120137
const yarn = isUsingYarn(sys);
121138
const stencil = coreCompiler.version || 'unknown';
122139
const system = `${sys.name} ${sys.version}`;
@@ -190,16 +207,16 @@ const CONFIG_PROPS_TO_DELETE: ReadonlyArray<keyof d.Config> = ['sys', 'logger',
190207
* @param config the config to anonymize
191208
* @returns an anonymized copy of the same config
192209
*/
193-
export const anonymizeConfigForTelemetry = (config: d.Config): d.Config => {
194-
const anonymizedConfig = { ...config };
210+
export const anonymizeConfigForTelemetry = (config: d.ValidatedConfig): d.Config => {
211+
const anonymizedConfig: d.Config = { ...config };
195212

196213
for (const prop of CONFIG_PROPS_TO_ANONYMIZE) {
197214
if (anonymizedConfig[prop] !== undefined) {
198215
anonymizedConfig[prop] = 'omitted';
199216
}
200217
}
201218

202-
anonymizedConfig.outputTargets = (config.outputTargets ?? []).map((target) => {
219+
anonymizedConfig.outputTargets = config.outputTargets.map((target) => {
203220
// Anonymize the outputTargets on our configuration, taking advantage of the
204221
// optional 2nd argument to `JSON.stringify`. If anything is not a string
205222
// we retain it so that any nested properties are handled, else we check

‎src/cli/telemetry/test/telemetry.spec.ts‎

Lines changed: 49 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -103,36 +103,42 @@ describe('checkTelemetry', () => {
103103
});
104104
});
105105

106-
describe('hasAppTarget', () => {
107-
it('Result is correct when outputTargets are empty', () => {
108-
const config = { outputTargets: [] } as d.Config;
106+
describe('hasAppTarget()', () => {
107+
let config: d.ValidatedConfig;
108+
let sys: d.CompilerSystem;
109+
110+
beforeEach(() => {
111+
sys = createSystem();
112+
config = mockValidatedConfig({ sys });
113+
});
114+
115+
it("returns 'false' when `outputTargets` is empty", () => {
116+
config.outputTargets = [];
109117
expect(telemetry.hasAppTarget(config)).toBe(false);
110118
});
111119

112-
it('Result is correct when outputTargets contains www with no baseUrl or serviceWorker', () => {
113-
const config = { outputTargets: [{ type: WWW }] } as d.Config;
120+
it("returns 'false' when `outputTargets` contains `www` with no `baseUrl` and no service worker", () => {
121+
config.outputTargets = [{ type: WWW }];
114122
expect(telemetry.hasAppTarget(config)).toBe(false);
115123
});
116124

117-
it('Result is correct when outputTargets contains www with default baseUrl value', () => {
118-
const config = { outputTargets: [{ type: WWW, baseUrl: '/' }] } as d.Config;
125+
it("returns 'false' when `outputTargets` contains `www` with '/' baseUrl value", () => {
126+
config.outputTargets = [{ type: WWW, baseUrl: '/' }];
119127
expect(telemetry.hasAppTarget(config)).toBe(false);
120128
});
121129

122-
it('Result is correct when outputTargets contains www with serviceWorker', () => {
123-
const config = { outputTargets: [{ type: WWW, serviceWorker: { swDest: './tmp' } }] } as d.Config;
130+
it("returns 'true' when `outputTargets` contains `www` with a service worker", () => {
131+
config.outputTargets = [{ type: WWW, serviceWorker: { swDest: './tmp' } }];
124132
expect(telemetry.hasAppTarget(config)).toBe(true);
125133
});
126134

127-
it('Result is correct when outputTargets contains www with baseUrl', () => {
128-
const config = { outputTargets: [{ type: WWW, baseUrl: 'https://example.com' }] } as d.Config;
135+
it("returns 'true' when `outputTargets` contains `www` with baseUrl", () => {
136+
config.outputTargets = [{ type: WWW, baseUrl: 'https://example.com' }];
129137
expect(telemetry.hasAppTarget(config)).toBe(true);
130138
});
131139

132-
it('Result is correct when outputTargets contains www with serviceWorker and baseUrl', () => {
133-
const config = {
134-
outputTargets: [{ type: WWW, baseUrl: 'https://example.com', serviceWorker: { swDest: './tmp' } }],
135-
} as d.Config;
140+
it("returns 'true' when `outputTargets` contains `www` with serviceWorker and baseUrl", () => {
141+
config.outputTargets = [{ type: WWW, baseUrl: 'https://example.com', serviceWorker: { swDest: './tmp' } }];
136142
expect(telemetry.hasAppTarget(config)).toBe(true);
137143
});
138144
});
@@ -277,6 +283,14 @@ describe('prepareData', () => {
277283
});
278284

279285
describe('anonymizeConfigForTelemetry', () => {
286+
let config: d.ValidatedConfig;
287+
let sys: d.CompilerSystem;
288+
289+
beforeEach(() => {
290+
sys = createSystem();
291+
config = mockValidatedConfig({ sys });
292+
});
293+
280294
it.each([
281295
'rootDir',
282296
'fsNamespace',
@@ -288,23 +302,28 @@ describe('anonymizeConfigForTelemetry', () => {
288302
'cacheDir',
289303
'configPath',
290304
'tsconfig',
291-
])("should anonymize top-level string prop '%s'", (prop: string) => {
292-
const anonymizedConfig = anonymizeConfigForTelemetry({ [prop]: "shouldn't see this!", outputTargets: [] });
293-
expect(anonymizedConfig).toEqual({ [prop]: 'omitted', outputTargets: [] });
305+
])("should anonymize top-level string prop '%s'", (prop: keyof d.ValidatedConfig) => {
306+
const anonymizedConfig = anonymizeConfigForTelemetry({
307+
...config,
308+
[prop]: "shouldn't see this!",
309+
outputTargets: [],
310+
});
311+
expect(anonymizedConfig[prop]).toBe('omitted');
312+
expect(anonymizedConfig.outputTargets).toEqual([]);
294313
});
295314

296315
it.each(['sys', 'logger', 'devServer', 'tsCompilerOptions'])(
297316
"should remove objects under prop '%s'",
298-
(prop: string) => {
299-
const anonymizedConfig = anonymizeConfigForTelemetry({ [prop]: {}, outputTargets: [] });
300-
expect(anonymizedConfig).toEqual({
301-
outputTargets: [],
302-
});
317+
(prop: keyof d.ValidatedConfig) => {
318+
const anonymizedConfig = anonymizeConfigForTelemetry({ ...config, [prop]: {}, outputTargets: [] });
319+
expect(anonymizedConfig.hasOwnProperty(prop)).toBe(false);
320+
expect(anonymizedConfig.outputTargets).toEqual([]);
303321
}
304322
);
305323

306324
it('should retain outputTarget props on the keep list', () => {
307325
const anonymizedConfig = anonymizeConfigForTelemetry({
326+
...config,
308327
outputTargets: [
309328
{ type: WWW, baseUrl: 'https://example.com' },
310329
{ type: DIST_HYDRATE_SCRIPT, external: ['beep', 'boop'], dir: 'shoud/go/away' },
@@ -314,14 +333,12 @@ describe('anonymizeConfigForTelemetry', () => {
314333
],
315334
});
316335

317-
expect(anonymizedConfig).toEqual({
318-
outputTargets: [
319-
{ type: WWW, baseUrl: 'omitted' },
320-
{ type: DIST_HYDRATE_SCRIPT, external: ['beep', 'boop'], dir: 'omitted' },
321-
{ type: DIST_CUSTOM_ELEMENTS, autoDefineCustomElements: false },
322-
{ type: DIST_CUSTOM_ELEMENTS, generateTypeDeclarations: true },
323-
{ type: DIST, typesDir: 'omitted' },
324-
],
325-
});
336+
expect(anonymizedConfig.outputTargets).toEqual([
337+
{ type: WWW, baseUrl: 'omitted' },
338+
{ type: DIST_HYDRATE_SCRIPT, external: ['beep', 'boop'], dir: 'omitted' },
339+
{ type: DIST_CUSTOM_ELEMENTS, autoDefineCustomElements: false },
340+
{ type: DIST_CUSTOM_ELEMENTS, generateTypeDeclarations: true },
341+
{ type: DIST, typesDir: 'omitted' },
342+
]);
326343
});
327344
});

0 commit comments

Comments
 (0)