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!)

Author: rwaskiewicz

src/cli/telemetry/telemetry.ts

@@ -82,7 +82,16 @@ export async function telemetryAction(
   }
 }
 
-export function hasAppTarget(config: d.Config): boolean {
+/**
+ * Helper function to determine if a Stencil configuration builds an application.
+ *
+ * This function is a rough approximation whether an application is generated as a part of a Stencil build, based on
+ * contents of the project's `stencil.config.ts` file.
+ *
+ * @param config the configuration used by the Stencil project
+ * @returns true if we believe the project generates an application, false otherwise
+ */
+export function hasAppTarget(config: d.ValidatedConfig): boolean {
   return config.outputTargets.some(
     (target) => target.type === WWW && (!!target.serviceWorker || (!!target.baseUrl && target.baseUrl !== '/'))
   );
@@ -92,7 +101,15 @@ export function isUsingYarn(sys: d.CompilerSystem) {
   return sys.getEnvironmentVar('npm_execpath')?.includes('yarn') || false;
 }
 
-export async function getActiveTargets(config: d.Config): Promise<string[]> {
+/**
+ * Build a list of the different types of output targets used in a Stencil configuration.
+ *
+ * Duplicate entries will not be returned from the list
+ *
+ * @param config the configuration used by the Stencil project
+ * @returns a unique list of output target types found in the Stencil configuration
+ */
+export function getActiveTargets(config: d.ValidatedConfig): string[] {
   const result = config.outputTargets.map((t) => t.type);
   return Array.from(new Set(result));
 }
@@ -116,7 +133,7 @@ export const prepareData = async (
 ): Promise<d.TrackableData> => {
   const { typescript, rollup } = coreCompiler.versions || { typescript: 'unknown', rollup: 'unknown' };
   const { packages, packagesNoVersions } = await getInstalledPackages(sys, config);
-  const targets = await getActiveTargets(config);
+  const targets = getActiveTargets(config);
   const yarn = isUsingYarn(sys);
   const stencil = coreCompiler.version || 'unknown';
   const system = `${sys.name} ${sys.version}`;
@@ -190,16 +207,16 @@ const CONFIG_PROPS_TO_DELETE: ReadonlyArray<keyof d.Config> = ['sys', 'logger',
  * @param config the config to anonymize
  * @returns an anonymized copy of the same config
  */
-export const anonymizeConfigForTelemetry = (config: d.Config): d.Config => {
-  const anonymizedConfig = { ...config };
+export const anonymizeConfigForTelemetry = (config: d.ValidatedConfig): d.Config => {
+  const anonymizedConfig: d.Config = { ...config };
 
   for (const prop of CONFIG_PROPS_TO_ANONYMIZE) {
     if (anonymizedConfig[prop] !== undefined) {
       anonymizedConfig[prop] = 'omitted';
     }
   }
 
-  anonymizedConfig.outputTargets = (config.outputTargets ?? []).map((target) => {
+  anonymizedConfig.outputTargets = config.outputTargets.map((target) => {
     // Anonymize the outputTargets on our configuration, taking advantage of the
     // optional 2nd argument to `JSON.stringify`. If anything is not a string
     // we retain it so that any nested properties are handled, else we check

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

@@ -103,36 +103,42 @@ describe('checkTelemetry', () => {
   });
 });
 
-describe('hasAppTarget', () => {
-  it('Result is correct when outputTargets are empty', () => {
-    const config = { outputTargets: [] } as d.Config;
+describe('hasAppTarget()', () => {
+  let config: d.ValidatedConfig;
+  let sys: d.CompilerSystem;
+
+  beforeEach(() => {
+    sys = createSystem();
+    config = mockValidatedConfig({ sys });
+  });
+
+  it("returns 'false' when `outputTargets` is empty", () => {
+    config.outputTargets = [];
     expect(telemetry.hasAppTarget(config)).toBe(false);
   });
 
-  it('Result is correct when outputTargets contains www with no baseUrl or serviceWorker', () => {
-    const config = { outputTargets: [{ type: WWW }] } as d.Config;
+  it("returns 'false' when `outputTargets` contains `www` with no `baseUrl` and no service worker", () => {
+    config.outputTargets = [{ type: WWW }];
     expect(telemetry.hasAppTarget(config)).toBe(false);
   });
 
-  it('Result is correct when outputTargets contains www with default baseUrl value', () => {
-    const config = { outputTargets: [{ type: WWW, baseUrl: '/' }] } as d.Config;
+  it("returns 'false' when `outputTargets` contains `www` with '/' baseUrl value", () => {
+    config.outputTargets = [{ type: WWW, baseUrl: '/' }];
     expect(telemetry.hasAppTarget(config)).toBe(false);
   });
 
-  it('Result is correct when outputTargets contains www with serviceWorker', () => {
-    const config = { outputTargets: [{ type: WWW, serviceWorker: { swDest: './tmp' } }] } as d.Config;
+  it("returns 'true' when `outputTargets` contains `www` with a service worker", () => {
+    config.outputTargets = [{ type: WWW, serviceWorker: { swDest: './tmp' } }];
     expect(telemetry.hasAppTarget(config)).toBe(true);
   });
 
-  it('Result is correct when outputTargets contains www with baseUrl', () => {
-    const config = { outputTargets: [{ type: WWW, baseUrl: 'https://example.com' }] } as d.Config;
+  it("returns 'true' when `outputTargets` contains `www` with baseUrl", () => {
+    config.outputTargets = [{ type: WWW, baseUrl: 'https://example.com' }];
     expect(telemetry.hasAppTarget(config)).toBe(true);
   });
 
-  it('Result is correct when outputTargets contains www with serviceWorker and baseUrl', () => {
-    const config = {
-      outputTargets: [{ type: WWW, baseUrl: 'https://example.com', serviceWorker: { swDest: './tmp' } }],
-    } as d.Config;
+  it("returns 'true' when `outputTargets` contains `www` with serviceWorker and baseUrl", () => {
+    config.outputTargets = [{ type: WWW, baseUrl: 'https://example.com', serviceWorker: { swDest: './tmp' } }];
     expect(telemetry.hasAppTarget(config)).toBe(true);
   });
 });
@@ -277,6 +283,14 @@ describe('prepareData', () => {
 });
 
 describe('anonymizeConfigForTelemetry', () => {
+  let config: d.ValidatedConfig;
+  let sys: d.CompilerSystem;
+
+  beforeEach(() => {
+    sys = createSystem();
+    config = mockValidatedConfig({ sys });
+  });
+
   it.each([
     'rootDir',
     'fsNamespace',
@@ -288,23 +302,28 @@ describe('anonymizeConfigForTelemetry', () => {
     'cacheDir',
     'configPath',
     'tsconfig',
-  ])("should anonymize top-level string prop '%s'", (prop: string) => {
-    const anonymizedConfig = anonymizeConfigForTelemetry({ [prop]: "shouldn't see this!", outputTargets: [] });
-    expect(anonymizedConfig).toEqual({ [prop]: 'omitted', outputTargets: [] });
+  ])("should anonymize top-level string prop '%s'", (prop: keyof d.ValidatedConfig) => {
+    const anonymizedConfig = anonymizeConfigForTelemetry({
+      ...config,
+      [prop]: "shouldn't see this!",
+      outputTargets: [],
+    });
+    expect(anonymizedConfig[prop]).toBe('omitted');
+    expect(anonymizedConfig.outputTargets).toEqual([]);
   });
 
   it.each(['sys', 'logger', 'devServer', 'tsCompilerOptions'])(
     "should remove objects under prop '%s'",
-    (prop: string) => {
-      const anonymizedConfig = anonymizeConfigForTelemetry({ [prop]: {}, outputTargets: [] });
-      expect(anonymizedConfig).toEqual({
-        outputTargets: [],
-      });
+    (prop: keyof d.ValidatedConfig) => {
+      const anonymizedConfig = anonymizeConfigForTelemetry({ ...config, [prop]: {}, outputTargets: [] });
+      expect(anonymizedConfig.hasOwnProperty(prop)).toBe(false);
+      expect(anonymizedConfig.outputTargets).toEqual([]);
     }
   );
 
   it('should retain outputTarget props on the keep list', () => {
     const anonymizedConfig = anonymizeConfigForTelemetry({
+      ...config,
       outputTargets: [
         { type: WWW, baseUrl: 'https://example.com' },
         { type: DIST_HYDRATE_SCRIPT, external: ['beep', 'boop'], dir: 'shoud/go/away' },
@@ -314,14 +333,12 @@ describe('anonymizeConfigForTelemetry', () => {
       ],
     });
 
-    expect(anonymizedConfig).toEqual({
-      outputTargets: [
-        { type: WWW, baseUrl: 'omitted' },
-        { type: DIST_HYDRATE_SCRIPT, external: ['beep', 'boop'], dir: 'omitted' },
-        { type: DIST_CUSTOM_ELEMENTS, autoDefineCustomElements: false },
-        { type: DIST_CUSTOM_ELEMENTS, generateTypeDeclarations: true },
-        { type: DIST, typesDir: 'omitted' },
-      ],
-    });
+    expect(anonymizedConfig.outputTargets).toEqual([
+      { type: WWW, baseUrl: 'omitted' },
+      { type: DIST_HYDRATE_SCRIPT, external: ['beep', 'boop'], dir: 'omitted' },
+      { type: DIST_CUSTOM_ELEMENTS, autoDefineCustomElements: false },
+      { type: DIST_CUSTOM_ELEMENTS, generateTypeDeclarations: true },
+      { type: DIST, typesDir: 'omitted' },
+    ]);
   });
 });