fix: remove the global polyfill for real

salesforce · Aug 22, 2023 · 4cafff8 · 4cafff8
1 parent 774d91b
commit 4cafff8
Show file tree

Hide file tree

Showing 13 changed files with 115 additions and 40 deletions.
diff --git a/packages/@lwc/engine-core/src/patches/detect-non-standard-aria.ts b/packages/@lwc/engine-core/src/patches/detect-non-standard-aria.ts
@@ -28,6 +28,11 @@ const NON_STANDARD_ARIA_PROPS = [
     'ariaOwns',
 ];
 
+function isGlobalAriaPolyfillLoaded(): boolean {
+    // sniff for the legacy polyfill being loaded
+    return !isUndefined(getOwnPropertyDescriptor(Element.prototype, 'ariaActiveDescendant'));
+}
+
 function findVM(elm: Element): VM | undefined {
     // If it's a shadow DOM component, then it has a host
     const { host } = elm.getRootNode() as ShadowRoot;
@@ -111,14 +116,12 @@ function enableDetection() {
 }
 
 // No point in running this code if we're not in a browser, or if the global polyfill is not loaded
-if (process.env.IS_BROWSER) {
-    if (lwcRuntimeFlags.ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL) {
-        // Always run detection in dev mode, so we can at least print to the console
-        if (process.env.NODE_ENV !== 'production') {
-            enableDetection();
-        } else {
-            // In prod mode, only enable detection if reporting is enabled
-            onReportingEnabled(enableDetection);
-        }
+if (process.env.IS_BROWSER && isGlobalAriaPolyfillLoaded()) {
+    // Always run detection in dev mode, so we can at least print to the console
+    if (process.env.NODE_ENV !== 'production') {
+        enableDetection();
+    } else {
+        // In prod mode, only enable detection if reporting is enabled
+        onReportingEnabled(enableDetection);
     }
 }
diff --git a/packages/@lwc/engine-dom/src/aria-reflection-polyfill.ts b/packages/@lwc/engine-dom/src/aria-reflection-polyfill.ts
diff --git a/packages/@lwc/engine-dom/src/index.ts b/packages/@lwc/engine-dom/src/index.ts
@@ -8,9 +8,6 @@
 // Globals -----------------------------------------------------------------------------------------
 import '@lwc/features';
 
-// Polyfills ---------------------------------------------------------------------------------------
-import './aria-reflection-polyfill';
-
 // DevTools Formatters
 import './formatters';
 

diff --git a/packages/@lwc/features/src/index.ts b/packages/@lwc/features/src/index.ts
@@ -17,7 +17,6 @@ const features: FeatureFlagMap = {
     ENABLE_WIRE_SYNC_EMIT: null,
     DISABLE_LIGHT_DOM_UNSCOPED_CSS: null,
     ENABLE_FROZEN_TEMPLATE: null,
-    ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL: null,
 };
 
 // eslint-disable-next-line no-restricted-properties

diff --git a/packages/@lwc/features/src/types.ts b/packages/@lwc/features/src/types.ts
@@ -59,12 +59,6 @@ export interface FeatureFlagMap {
      * ```
      */
     ENABLE_FROZEN_TEMPLATE: FeatureFlagValue;
-
-    /**
-     * Flag to enable the ARIA reflection polyfill. When set to true, ARIA reflection will be applied globally
-     * to Element.prototype, not just to the LightningElement and HTMLBridgeElement base classes.
-     */
-    ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL: FeatureFlagValue;
 }
 
 export type FeatureFlagName = keyof FeatureFlagMap;
diff --git a/...es/@lwc/integration-karma/scripts/karma-configs/shared/aria-reflection-global-polyfill.js b/...es/@lwc/integration-karma/scripts/karma-configs/shared/aria-reflection-global-polyfill.js
@@ -0,0 +1,76 @@
+// Minimal polyfill of ARIA string reflection, plus some non-standard ARIA props
+// Taken from https://github.com/salesforce/lwc/blob/44a01ef/packages/%40lwc/shared/src/aria.ts#L22-L70
+// This should be analogous to the same one used in lwc-platform
+const ARIA_PROPERTIES = [
+    'ariaActiveDescendant',
+    'ariaAtomic',
+    'ariaAutoComplete',
+    'ariaBusy',
+    'ariaChecked',
+    'ariaColCount',
+    'ariaColIndex',
+    'ariaColSpan',
+    'ariaControls',
+    'ariaCurrent',
+    'ariaDescribedBy',
+    'ariaDetails',
+    'ariaDisabled',
+    'ariaErrorMessage',
+    'ariaExpanded',
+    'ariaFlowTo',
+    'ariaHasPopup',
+    'ariaHidden',
+    'ariaInvalid',
+    'ariaKeyShortcuts',
+    'ariaLabel',
+    'ariaLabelledBy',
+    'ariaLevel',
+    'ariaLive',
+    'ariaModal',
+    'ariaMultiLine',
+    'ariaMultiSelectable',
+    'ariaOrientation',
+    'ariaOwns',
+    'ariaPlaceholder',
+    'ariaPosInSet',
+    'ariaPressed',
+    'ariaReadOnly',
+    'ariaRelevant',
+    'ariaRequired',
+    'ariaRoleDescription',
+    'ariaRowCount',
+    'ariaRowIndex',
+    'ariaRowSpan',
+    'ariaSelected',
+    'ariaSetSize',
+    'ariaSort',
+    'ariaValueMax',
+    'ariaValueMin',
+    'ariaValueNow',
+    'ariaValueText',
+    'role',
+];
+
+for (const prop of ARIA_PROPERTIES) {
+    const attribute = prop.replace(/^aria/, 'aria-').toLowerCase(); // e.g. ariaPosInSet => aria-posinset
+
+    if (!Object.getOwnPropertyDescriptor(Element.prototype, prop)) {
+        Object.defineProperty(Element.prototype, prop, {
+            get() {
+                return this.getAttribute(attribute);
+            },
+            set(value) {
+                // Per the spec, only null is treated as removing the attribute. However, Chromium/WebKit currently
+                // differ from the spec and allow undefined as well. Here, we follow the spec, as well as
+                // @lwc/aria-reflection's historical behavior. See: https://github.com/w3c/aria/issues/1858
+                if (value === null) {
+                    this.removeAttribute(attribute);
+                } else {
+                    this.setAttribute(attribute, value);
+                }
+            },
+            configurable: true,
+            enumerable: true,
+        });
+    }
+}
diff --git a/packages/@lwc/integration-karma/scripts/karma-configs/test/base.js b/packages/@lwc/integration-karma/scripts/karma-configs/test/base.js
@@ -12,7 +12,12 @@ const path = require('path');
 const karmaPluginLwc = require('../../karma-plugins/lwc');
 const karmaPluginEnv = require('../../karma-plugins/env');
 const karmaPluginTransformFramework = require('../../karma-plugins/transform-framework.js');
-const { SYNTHETIC_SHADOW_ENABLED, GREP, COVERAGE } = require('../../shared/options');
+const {
+    SYNTHETIC_SHADOW_ENABLED,
+    GREP,
+    COVERAGE,
+    ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL,
+} = require('../../shared/options');
 const { createPattern } = require('../utils');
 const TAGS = require('./tags');
 
@@ -22,12 +27,20 @@ const COVERAGE_DIR = path.resolve(__dirname, '../../../coverage');
 const SYNTHETIC_SHADOW = require.resolve('@lwc/synthetic-shadow/dist/index.js');
 const LWC_ENGINE = require.resolve('@lwc/engine-dom/dist/index.js');
 const WIRE_SERVICE = require.resolve('@lwc/wire-service/dist/index.js');
+const ARIA_REFLECTION_GLOBAL_POLYFILL = require.resolve(
+    '../shared/aria-reflection-global-polyfill.js'
+);
 
 const TEST_UTILS = require.resolve('../../../helpers/test-utils');
 const WIRE_SETUP = require.resolve('../../../helpers/wire-setup');
 const TEST_SETUP = require.resolve('../../../helpers/test-setup');
 
-const ALL_FRAMEWORK_FILES = [SYNTHETIC_SHADOW, LWC_ENGINE, WIRE_SERVICE];
+const ALL_FRAMEWORK_FILES = [
+    SYNTHETIC_SHADOW,
+    LWC_ENGINE,
+    WIRE_SERVICE,
+    ARIA_REFLECTION_GLOBAL_POLYFILL,
+];
 
 // Fix Node warning about >10 event listeners ("Possible EventEmitter memory leak detected").
 // This is due to the fact that we are running so many simultaneous rollup commands
@@ -41,6 +54,9 @@ function getFiles() {
     if (SYNTHETIC_SHADOW_ENABLED) {
         frameworkFiles.push(createPattern(SYNTHETIC_SHADOW));
     }
+    if (ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL) {
+        frameworkFiles.push(createPattern(ARIA_REFLECTION_GLOBAL_POLYFILL));
+    }
     frameworkFiles.push(createPattern(LWC_ENGINE));
     frameworkFiles.push(createPattern(WIRE_SERVICE));
     frameworkFiles.push(createPattern(WIRE_SETUP));

diff --git a/packages/@lwc/integration-karma/scripts/karma-plugins/env.js b/packages/@lwc/integration-karma/scripts/karma-plugins/env.js
@@ -37,8 +37,7 @@ function createEnvFile() {
         `
         window.lwcRuntimeFlags = {
             ENABLE_FORCE_NATIVE_SHADOW_MODE_FOR_TEST: ${FORCE_NATIVE_SHADOW_MODE_FOR_TEST},
-            ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE: ${ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE},
-            ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL: ${ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL}
+            ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE: ${ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE}
         };
         window.process = {
             env: {
@@ -47,6 +46,7 @@ function createEnvFile() {
                 NATIVE_SHADOW: ${!SYNTHETIC_SHADOW_ENABLED || FORCE_NATIVE_SHADOW_MODE_FOR_TEST},
                 NATIVE_SHADOW_ROOT_DEFINED: typeof ShadowRoot !== 'undefined',
                 SYNTHETIC_SHADOW_ENABLED: ${SYNTHETIC_SHADOW_ENABLED},
+                ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL: ${ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL},
                 LWC_VERSION: ${JSON.stringify(LWC_VERSION)},
                 API_VERSION: ${JSON.stringify(API_VERSION)}
             }

diff --git a/packages/@lwc/integration-karma/scripts/karma-plugins/transform-framework.js b/packages/@lwc/integration-karma/scripts/karma-plugins/transform-framework.js
@@ -24,6 +24,9 @@ function getIifeName(filename) {
     } else if (filename.includes('@lwc/synthetic-shadow')) {
         // synthetic shadow does not need an IIFE name
         return undefined;
+    } else if (filename.includes('aria-reflection-global-polyfill.js')) {
+        // aria reflection global polyfill does not need an IIFE name
+        return undefined;
     }
     throw new Error(`Unknown framework filename, not sure which IIFE name to use: ${filename}`);
 }

diff --git a/packages/@lwc/integration-karma/test/accessibility/non-standard-aria-props/index.spec.js b/packages/@lwc/integration-karma/test/accessibility/non-standard-aria-props/index.spec.js
@@ -4,7 +4,7 @@ import Light from 'x/light';
 import Shadow from 'x/shadow';
 
 // This test only works if the ARIA reflection polyfill is loaded
-if (window.lwcRuntimeFlags.ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL) {
+if (process.env.ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL) {
     describe('non-standard ARIA properties', () => {
         let dispatcher;
 

diff --git a/packages/@lwc/integration-karma/test/accessibility/synthetic-cross-root-aria/index.spec.js b/packages/@lwc/integration-karma/test/accessibility/synthetic-cross-root-aria/index.spec.js
@@ -41,7 +41,7 @@ if (!process.env.NATIVE_SHADOW) {
 
             // It doesn't make sense to test setting e.g. `elm.ariaLabelledBy` if the global
             // polyfill is not applied
-            if (window.lwcRuntimeFlags.ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL) {
+            if (process.env.ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL) {
                 usePropertyAccessValues.push(true);
             }
 

diff --git a/packages/@lwc/integration-karma/test/polyfills/aria-properties/index.spec.js b/packages/@lwc/integration-karma/test/polyfills/aria-properties/index.spec.js
@@ -166,7 +166,7 @@ function testAriaProperty(property, attribute) {
 }
 
 // These tests don't make sense if the global polyfill is not loaded
-if (window.lwcRuntimeFlags.ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL) {
+if (process.env.ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL) {
     for (const [ariaProperty, ariaAttribute] of Object.entries(ariaPropertiesMapping)) {
         testAriaProperty(ariaProperty, ariaAttribute);
     }

diff --git a/packages/@lwc/integration-karma/test/template/attribute-aria/index.spec.js b/packages/@lwc/integration-karma/test/template/attribute-aria/index.spec.js
@@ -74,7 +74,7 @@ describe('setting aria attributes', () => {
 
         // If the polyfill is not enabled, then we can't expect the div to have all the ARIA properties,
         // because some are non-standard or not supported in all browsers
-        if (window.lwcRuntimeFlags.ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL) {
+        if (process.env.ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL) {
             it('aria prop is set', () => {
                 for (const prop of ariaProperties) {
                     expect(childComponent[prop]).toMatch(/^foo/);