fix(compiler): restrict config extras slot fix flags (#4767)

tanner-reits · web-flow · commit f2c322959c13 · 2023-09-12T14:08:55.000Z
* setup config types

* update config validation logic

* resolve build errors

* add warning to validation logic

* use initial extras values for warning message check

* update config types to allow explicit values

* update validation check

* pr feedback

* update warning message language
diff --git a/src/compiler/config/test/validate-config.spec.ts b/src/compiler/config/test/validate-config.spec.ts
@@ -405,13 +405,15 @@ describe('validation', () => {
   });
 
   it('should override slot fix config based on `experimentalSlotFixes`', () => {
+    // This test is to verify the flags get overwritten correctly even if an
+    // invalid config is ingested. Hence, the `any` cast
     userConfig.extras = {
       appendChildSlotFix: false,
       slotChildNodesFix: false,
       cloneNodeFix: false,
       scopedSlotTextContentFix: false,
-    };
-    userConfig.extras.experimentalSlotFixes = true;
+      experimentalSlotFixes: true,
+    } as any;
     const { config } = validateConfig(userConfig, bootstrapConfig);
     expect(config.extras.appendChildSlotFix).toBe(true);
     expect(config.extras.cloneNodeFix).toBe(true);
diff --git a/src/compiler/config/validate-config.ts b/src/compiler/config/validate-config.ts
@@ -1,7 +1,14 @@
 import { createNodeLogger, createNodeSys } from '@sys-api-node';
 import { buildError, isBoolean, isNumber, isString, sortBy } from '@utils';
 
-import { ConfigBundle, Diagnostic, LoadConfigInit, UnvalidatedConfig, ValidatedConfig } from '../../declarations';
+import {
+  ConfigBundle,
+  ConfigExtras,
+  Diagnostic,
+  LoadConfigInit,
+  UnvalidatedConfig,
+  ValidatedConfig,
+} from '../../declarations';
 import { setBooleanConfig } from './config-utils';
 import { validateOutputTargets } from './outputs';
 import { validateDevServer } from './validate-dev-server';
@@ -104,24 +111,43 @@ export const validateConfig = (
   }
 
   validatedConfig.extras = validatedConfig.extras || {};
-  validatedConfig.extras.appendChildSlotFix = !!validatedConfig.extras.appendChildSlotFix;
-  validatedConfig.extras.cloneNodeFix = !!validatedConfig.extras.cloneNodeFix;
   validatedConfig.extras.lifecycleDOMEvents = !!validatedConfig.extras.lifecycleDOMEvents;
   validatedConfig.extras.scriptDataOpts = !!validatedConfig.extras.scriptDataOpts;
-  validatedConfig.extras.slotChildNodesFix = !!validatedConfig.extras.slotChildNodesFix;
   validatedConfig.extras.initializeNextTick = !!validatedConfig.extras.initializeNextTick;
   validatedConfig.extras.tagNameTransform = !!validatedConfig.extras.tagNameTransform;
-  // TODO(STENCIL-914): remove `experimentalSlotFixes` when it's the default behavior
-  validatedConfig.extras.experimentalSlotFixes = !!validatedConfig.extras.experimentalSlotFixes;
 
-  // if the user has set `extras.experimentalSlotFixes` then we turn on all of
-  // the slot-related fixes automatically.
+  // TODO(STENCIL-914): remove when `experimentalSlotFixes` is the default behavior
+  // If the user set `experimentalSlotFixes` and any individual slot fix flags to `false`, we need to log a warning
+  // to the user that we will "override" the individual flags
+  if (validatedConfig.extras.experimentalSlotFixes === true) {
+    const possibleFlags: (keyof ConfigExtras)[] = [
+      'appendChildSlotFix',
+      'slotChildNodesFix',
+      'cloneNodeFix',
+      'scopedSlotTextContentFix',
+    ];
+    const conflictingFlags = possibleFlags.filter((flag) => validatedConfig.extras[flag] === false);
+    if (conflictingFlags.length > 0) {
+      const warning = buildError(diagnostics);
+      warning.level = 'warn';
+      warning.messageText = `If the 'experimentalSlotFixes' flag is enabled it will override any slot fix flags which are disabled. In particular, the following currently-disabled flags will be ignored: ${conflictingFlags.join(
+        ', ',
+      )}. Please update your Stencil config accordingly.`;
+    }
+  }
+
   // TODO(STENCIL-914): remove `experimentalSlotFixes` when it's the default behavior
-  if (validatedConfig.extras.experimentalSlotFixes) {
+  validatedConfig.extras.experimentalSlotFixes = !!validatedConfig.extras.experimentalSlotFixes;
+  if (validatedConfig.extras.experimentalSlotFixes === true) {
     validatedConfig.extras.appendChildSlotFix = true;
     validatedConfig.extras.cloneNodeFix = true;
     validatedConfig.extras.slotChildNodesFix = true;
     validatedConfig.extras.scopedSlotTextContentFix = true;
+  } else {
+    validatedConfig.extras.appendChildSlotFix = !!validatedConfig.extras.appendChildSlotFix;
+    validatedConfig.extras.cloneNodeFix = !!validatedConfig.extras.cloneNodeFix;
+    validatedConfig.extras.slotChildNodesFix = !!validatedConfig.extras.slotChildNodesFix;
+    validatedConfig.extras.scopedSlotTextContentFix = !!validatedConfig.extras.scopedSlotTextContentFix;
   }
 
   validatedConfig.buildEs5 =
diff --git a/src/declarations/stencil-public-compiler.ts b/src/declarations/stencil-public-compiler.ts
@@ -292,27 +292,7 @@ export interface StencilConfig {
   stencilCoreResolvedId?: string;
 }
 
-export interface ConfigExtras {
-  // TODO(STENCIL-914): remove this option when `experimentalSlotFixes` is the default behavior
-  /**
-   * By default, the slot polyfill does not update `appendChild()` so that it appends
-   * new child nodes into the correct child slot like how shadow dom works. This is an opt-in
-   * polyfill for those who need it when using `element.appendChild(node)` and expecting the
-   * child to be appended in the same location shadow dom would. This is not required for
-   * IE11 or Edge 18, but can be enabled if the app is using `appendChild()`. Defaults to `false`.
-   */
-  appendChildSlotFix?: boolean;
-
-  // TODO(STENCIL-914): remove this option when `experimentalSlotFixes` is the default behavior
-  /**
-   * By default, the runtime does not polyfill `cloneNode()` when cloning a component
-   * that uses the slot polyfill. This is an opt-in polyfill for those who need it.
-   * This is not required for IE11 or Edge 18, but can be enabled if the app is using
-   * `cloneNode()` and unexpected node are being cloned due to the slot polyfill
-   * simulating shadow dom. Defaults to `false`.
-   */
-  cloneNodeFix?: boolean;
-
+interface ConfigExtrasBase {
   /**
    * Experimental flag. Projects that use a Stencil library built using the `dist` output target may have trouble lazily
    * loading components when using a bundler such as Vite or Parcel. Setting this flag to `true` will change how Stencil
@@ -343,13 +323,6 @@ export interface ConfigExtras {
    */
   scriptDataOpts?: boolean;
 
-  // TODO(STENCIL-914): remove this option when `experimentalSlotFixes` is the default behavior
-  /**
-   * Experimental flag to align the behavior of invoking `textContent` on a scoped component to act more like a
-   * component that uses the shadow DOM. Defaults to `false`
-   */
-  scopedSlotTextContentFix?: boolean;
-
   /**
    * When a component is first attached to the DOM, this setting will wait a single tick before
    * rendering. This works around an Angular issue, where Angular attaches the elements before
@@ -358,28 +331,61 @@ export interface ConfigExtras {
    */
   initializeNextTick?: boolean;
 
+  /**
+   * Enables the tagNameTransform option of `defineCustomElements()`, so the component tagName
+   * can be customized at runtime. Defaults to `false`.
+   */
+  tagNameTransform?: boolean;
+}
+
+// TODO(STENCIL-914): delete this interface when `experimentalSlotFixes` is the default behavior
+type ConfigExtrasSlotFixes<ExperimentalFixesEnabled extends boolean, IndividualFlags extends boolean> = {
+  // TODO(STENCIL-914): remove this option when `experimentalSlotFixes` is the default behavior
+  /**
+   * By default, the slot polyfill does not update `appendChild()` so that it appends
+   * new child nodes into the correct child slot like how shadow dom works. This is an opt-in
+   * polyfill for those who need it when using `element.appendChild(node)` and expecting the
+   * child to be appended in the same location shadow dom would. This is not required for
+   * IE11 or Edge 18, but can be enabled if the app is using `appendChild()`. Defaults to `false`.
+   */
+  appendChildSlotFix?: IndividualFlags;
+
+  // TODO(STENCIL-914): remove this option when `experimentalSlotFixes` is the default behavior
+  /**
+   * By default, the runtime does not polyfill `cloneNode()` when cloning a component
+   * that uses the slot polyfill. This is an opt-in polyfill for those who need it.
+   * This is not required for IE11 or Edge 18, but can be enabled if the app is using
+   * `cloneNode()` and unexpected node are being cloned due to the slot polyfill
+   * simulating shadow dom. Defaults to `false`.
+   */
+  cloneNodeFix?: IndividualFlags;
+
+  // TODO(STENCIL-914): remove this option when `experimentalSlotFixes` is the default behavior
+  /**
+   * Experimental flag to align the behavior of invoking `textContent` on a scoped component to act more like a
+   * component that uses the shadow DOM. Defaults to `false`
+   */
+  scopedSlotTextContentFix?: IndividualFlags;
+
   // TODO(STENCIL-914): remove this option when `experimentalSlotFixes` is the default behavior
   /**
    * For browsers that do not support shadow dom (IE11 and Edge 18 and below), slot is polyfilled
    * to simulate the same behavior. However, the host element's `childNodes` and `children`
    * getters are not patched to only show the child nodes and elements of the default slot.
    * Defaults to `false`.
    */
-  slotChildNodesFix?: boolean;
-
-  /**
-   * Enables the tagNameTransform option of `defineCustomElements()`, so the component tagName
-   * can be customized at runtime. Defaults to `false`.
-   */
-  tagNameTransform?: boolean;
+  slotChildNodesFix?: IndividualFlags;
 
   // TODO(STENCIL-914): remove `experimentalSlotFixes` when it's the default behavior
   /**
    * Enables all slot-related fixes such as {@link slotChildNodesFix}, and
    * {@link scopedSlotTextContentFix}.
    */
-  experimentalSlotFixes?: boolean;
-}
+  experimentalSlotFixes?: ExperimentalFixesEnabled;
+};
+
+export type ConfigExtras = ConfigExtrasBase &
+  (ConfigExtrasSlotFixes<true, true> | ConfigExtrasSlotFixes<false, boolean>);
 
 export interface Config extends StencilConfig {
   buildAppCore?: boolean;
