From 6c913e1d485127f6c1e9953218a7ff309acd01a0 Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Tue, 8 Aug 2023 15:09:55 -0700 Subject: [PATCH 01/15] fix(engine-dom): disable ARIA reflection polyfill (BREAKING CHANGE) --- .circleci/config.yml | 12 ++-- .../src/framework/base-bridge-element.ts | 18 +++--- .../src/framework/base-lightning-element.ts | 14 ++--- .../src/patches/detect-non-standard-aria.ts | 56 ++++++++----------- .../src/aria-reflection-polyfill.ts | 4 +- .../src/__tests__/fixtures.spec.ts | 4 -- packages/@lwc/features/src/index.ts | 2 +- packages/@lwc/features/src/types.ts | 4 +- packages/@lwc/integration-karma/README.md | 2 +- .../scripts/karma-configs/test/tags.js | 4 +- .../scripts/karma-plugins/env.js | 4 +- .../scripts/shared/options.js | 6 +- .../non-standard-aria-props/index.spec.js | 2 +- .../synthetic-cross-root-aria/index.spec.js | 10 +++- .../polyfills/aria-properties/index.spec.js | 2 +- .../template/attribute-aria/index.spec.js | 2 +- 16 files changed, 70 insertions(+), 76 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index f3202ed89c..c37216570b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -61,7 +61,7 @@ commands: enable_native_custom_element_lifecycle: type: boolean default: false - disable_aria_reflection_polyfill: + enable_aria_reflection_polyfill: type: boolean default: false node_env_production: @@ -95,7 +95,7 @@ commands: <<# parameters.disable_synthetic >> DISABLE_SYNTHETIC=1 <> \ <<# parameters.force_native_shadow_mode >> FORCE_NATIVE_SHADOW_MODE_FOR_TEST=1 <> \ <<# parameters.enable_native_custom_element_lifecycle >> ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE=1 <> \ - <<# parameters.disable_aria_reflection_polyfill >> DISABLE_ARIA_REFLECTION_POLYFILL=1 <> \ + <<# parameters.enable_aria_reflection_polyfill >> ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL=1 <> \ <<# parameters.node_env_production >> NODE_ENV_FOR_TEST=production <> \ <<# parameters.disable_synthetic_shadow_support_in_compiler >> DISABLE_SYNTHETIC_SHADOW_SUPPORT_IN_COMPILER=1 <> \ <<# parameters.legacy_browsers >> LEGACY_BROWSERS=1 <> \ @@ -188,7 +188,7 @@ commands: enable_native_custom_element_lifecycle: type: boolean default: false - disable_aria_reflection_polyfill: + enable_aria_reflection_polyfill: type: boolean default: false node_env_production: @@ -210,7 +210,7 @@ commands: disable_synthetic: << parameters.disable_synthetic >> force_native_shadow_mode: << parameters.force_native_shadow_mode >> enable_native_custom_element_lifecycle: << parameters.enable_native_custom_element_lifecycle >> - disable_aria_reflection_polyfill: << parameters.disable_aria_reflection_polyfill >> + enable_aria_reflection_polyfill: << parameters.enable_aria_reflection_polyfill >> node_env_production: << parameters.node_env_production >> disable_synthetic_shadow_support_in_compiler: << parameters.disable_synthetic_shadow_support_in_compiler >> api_version: << parameters.api_version >> @@ -298,9 +298,11 @@ jobs: - run_karma: disable_synthetic: true api_version: 58 + - run_karma: + enable_aria_reflection_polyfill: true - run_karma: disable_synthetic: true - disable_aria_reflection_polyfill: true + enable_aria_reflection_polyfill: true - run_karma: disable_synthetic: true disable_synthetic_shadow_support_in_compiler: true diff --git a/packages/@lwc/engine-core/src/framework/base-bridge-element.ts b/packages/@lwc/engine-core/src/framework/base-bridge-element.ts index 9cf7694990..80168c8e1a 100644 --- a/packages/@lwc/engine-core/src/framework/base-bridge-element.ts +++ b/packages/@lwc/engine-core/src/framework/base-bridge-element.ts @@ -169,15 +169,15 @@ if (process.env.IS_BROWSER) { // This ARIA reflection only really makes sense in the browser. On the server, there is no `renderedCallback()`, // so you cannot do e.g. `this.template.querySelector('x-child').ariaBusy = 'true'`. So we don't need to expose // ARIA props outside the LightningElement - if (lwcRuntimeFlags.DISABLE_ARIA_REFLECTION_POLYFILL) { - // If ARIA reflection is not applied globally to Element.prototype, apply it to HTMLBridgeElement.prototype. - // This allows `elm.aria*` property accessors to work from outside a component, and to reflect `aria-*` attrs. - // This is especially important because the template compiler compiles aria-* attrs on components to aria* props - // - // Also note that we apply this to BaseBridgeElement.prototype to avoid excessively redefining property - // accessors inside the HTMLBridgeElementFactory. - applyAriaReflection(BaseBridgeElement.prototype); - } + // + // Apply ARIA reflection to HTMLBridgeElement.prototype. This allows `elm.aria*` property accessors to work from + // outside a component, and to reflect `aria-*` attrs. This is especially important because the template compiler + // compiles aria-* attrs on components to aria* props. + // Note this works regardless of whether the global ARIA reflection polyfill is applied or not. + // + // Also note that we apply this to BaseBridgeElement.prototype to avoid excessively redefining property + // accessors inside the HTMLBridgeElementFactory. + applyAriaReflection(BaseBridgeElement.prototype); } freeze(BaseBridgeElement); diff --git a/packages/@lwc/engine-core/src/framework/base-lightning-element.ts b/packages/@lwc/engine-core/src/framework/base-lightning-element.ts index ed28295588..92d984ae2b 100644 --- a/packages/@lwc/engine-core/src/framework/base-lightning-element.ts +++ b/packages/@lwc/engine-core/src/framework/base-lightning-element.ts @@ -703,16 +703,10 @@ for (const propName in HTMLElementOriginalDescriptors) { defineProperties(LightningElement.prototype, lightningBasedDescriptors); -function applyAriaReflectionToLightningElement() { - // If ARIA reflection is not applied globally to Element.prototype, or if we are running server-side, - // apply it to LightningElement.prototype. - // This allows `this.aria*` property accessors to work from inside a component, and to reflect `aria-*` attrs. - applyAriaReflection(LightningElement.prototype); -} - -if (!process.env.IS_BROWSER || lwcRuntimeFlags.DISABLE_ARIA_REFLECTION_POLYFILL) { - applyAriaReflectionToLightningElement(); -} +// Apply ARIA reflection to LightningElement.prototype, on both the browser and server. +// This allows `this.aria*` property accessors to work from inside a component, and to reflect `aria-*` attrs. +// Note this works regardless of whether the global ARIA reflection polyfill is applied or not. +applyAriaReflection(LightningElement.prototype); defineProperty(LightningElement, 'CustomElementConstructor', { get() { 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 index 61be617283..8eddce0402 100644 --- a/packages/@lwc/engine-core/src/patches/detect-non-standard-aria.ts +++ b/packages/@lwc/engine-core/src/patches/detect-non-standard-aria.ts @@ -7,7 +7,6 @@ import { defineProperty, getOwnPropertyDescriptor, isNull, isUndefined } from '@lwc/shared'; import { onReportingEnabled, report, ReportingEventId } from '../framework/reporting'; -import { LightningElement } from '../framework/base-lightning-element'; import { logWarnOnce } from '../shared/logger'; import { getAssociatedVMIfPresent, VM } from '../framework/vm'; import { BaseBridgeElement } from '../framework/base-bridge-element'; @@ -29,12 +28,6 @@ const NON_STANDARD_ARIA_PROPS = [ 'ariaOwns', ]; -function isLightningElement(elm: Element) { - // The former case is for `this.prop` (inside component) and the latter is for `element.prop` (outside component). - // In both cases, we apply the non-standard prop even when the global polyfill is disabled, so this is kosher. - return elm instanceof LightningElement || elm instanceof BaseBridgeElement; -} - function findVM(elm: Element): VM | undefined { // If it's a shadow DOM component, then it has a host const { host } = elm.getRootNode() as ShadowRoot; @@ -45,7 +38,8 @@ function findVM(elm: Element): VM | undefined { // Else it might be a light DOM component. Walk up the tree trying to find the owner let parentElement: Element | null = elm; while (!isNull((parentElement = parentElement.parentElement))) { - if (isLightningElement(parentElement)) { + if (parentElement instanceof BaseBridgeElement) { + // parentElement is an LWC component const vm = getAssociatedVMIfPresent(parentElement); if (!isUndefined(vm)) { return vm; @@ -56,32 +50,30 @@ function findVM(elm: Element): VM | undefined { } function checkAndReportViolation(elm: Element, prop: string, isSetter: boolean, setValue: any) { - if (!isLightningElement(elm)) { - const vm = findVM(elm); + const vm = findVM(elm); - if (process.env.NODE_ENV !== 'production') { - logWarnOnce( - `Element <${elm.tagName.toLowerCase()}> ` + - (isUndefined(vm) ? '' : `owned by <${vm.elm.tagName.toLowerCase()}> `) + - `uses non-standard property "${prop}". This will be removed in a future version of LWC. ` + - `See https://sfdc.co/deprecated-aria` - ); - } + if (process.env.NODE_ENV !== 'production') { + logWarnOnce( + `Element <${elm.tagName.toLowerCase()}> ` + + (isUndefined(vm) ? '' : `owned by <${vm.elm.tagName.toLowerCase()}> `) + + `uses non-standard property "${prop}". This will be removed in a future version of LWC. ` + + `See https://sfdc.co/deprecated-aria` + ); + } - let setValueType: string | undefined; - if (isSetter) { - // `typeof null` is "object" which is not very useful for detecting null. - // We mostly want to know null vs undefined vs other types here, due to - // https://github.com/salesforce/lwc/issues/3284 - setValueType = isNull(setValue) ? 'null' : typeof setValue; - } - report(ReportingEventId.NonStandardAriaReflection, { - tagName: vm?.tagName, - propertyName: prop, - isSetter, - setValueType, - }); + let setValueType: string | undefined; + if (isSetter) { + // `typeof null` is "object" which is not very useful for detecting null. + // We mostly want to know null vs undefined vs other types here, due to + // https://github.com/salesforce/lwc/issues/3284 + setValueType = isNull(setValue) ? 'null' : typeof setValue; } + report(ReportingEventId.NonStandardAriaReflection, { + tagName: vm?.tagName, + propertyName: prop, + isSetter, + setValueType, + }); } function enableDetection() { @@ -120,7 +112,7 @@ 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.DISABLE_ARIA_REFLECTION_POLYFILL) { + 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(); diff --git a/packages/@lwc/engine-dom/src/aria-reflection-polyfill.ts b/packages/@lwc/engine-dom/src/aria-reflection-polyfill.ts index b3aebab5fa..4db542dfa9 100644 --- a/packages/@lwc/engine-dom/src/aria-reflection-polyfill.ts +++ b/packages/@lwc/engine-dom/src/aria-reflection-polyfill.ts @@ -6,8 +6,8 @@ */ import { applyAriaReflection } from '@lwc/aria-reflection'; -if (!lwcRuntimeFlags.DISABLE_ARIA_REFLECTION_POLYFILL) { - // If DISABLE_ARIA_REFLECTION_POLYFILL is false, then we need to apply the ARIA reflection polyfill globally, +if (lwcRuntimeFlags.ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL) { + // If ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL is true, then we need to apply the ARIA reflection polyfill globally, // i.e. to the global Element.prototype applyAriaReflection(); } diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures.spec.ts b/packages/@lwc/engine-server/src/__tests__/fixtures.spec.ts index e1de9734c7..b30be8bb89 100755 --- a/packages/@lwc/engine-server/src/__tests__/fixtures.spec.ts +++ b/packages/@lwc/engine-server/src/__tests__/fixtures.spec.ts @@ -228,8 +228,4 @@ describe('fixtures', () => { describe('native custom element lifecycle', () => { testWithFeatureFlagEnabled('ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE'); }); - - describe('disable aria reflection polyfill', () => { - testWithFeatureFlagEnabled('DISABLE_ARIA_REFLECTION_POLYFILL'); - }); }); diff --git a/packages/@lwc/features/src/index.ts b/packages/@lwc/features/src/index.ts index a326809320..c0777abdb4 100644 --- a/packages/@lwc/features/src/index.ts +++ b/packages/@lwc/features/src/index.ts @@ -17,7 +17,7 @@ const features: FeatureFlagMap = { ENABLE_WIRE_SYNC_EMIT: null, DISABLE_LIGHT_DOM_UNSCOPED_CSS: null, ENABLE_FROZEN_TEMPLATE: null, - DISABLE_ARIA_REFLECTION_POLYFILL: 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 index 6f2156c300..b761710d57 100644 --- a/packages/@lwc/features/src/types.ts +++ b/packages/@lwc/features/src/types.ts @@ -61,11 +61,11 @@ export interface FeatureFlagMap { ENABLE_FROZEN_TEMPLATE: FeatureFlagValue; /** - * Flag to remove the ARIA reflection polyfill. When set to true, this flag will avoid the global DOM patching + * Flag to enable the ARIA reflection polyfill. When set to false, this flag will avoid the global DOM patching * to polyfill ARIA reflection. Instead, the necessary ARIA properties will only exist on the LightningElement * and HTMLBridgeElement base classes, not on every Element. */ - DISABLE_ARIA_REFLECTION_POLYFILL: FeatureFlagValue; + ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL: FeatureFlagValue; } export type FeatureFlagName = keyof FeatureFlagMap; diff --git a/packages/@lwc/integration-karma/README.md b/packages/@lwc/integration-karma/README.md index f637f65def..984a026182 100644 --- a/packages/@lwc/integration-karma/README.md +++ b/packages/@lwc/integration-karma/README.md @@ -34,7 +34,7 @@ This set of environment variables applies to the `start` and `test` commands: - **`DISABLE_SYNTHETIC=1`:** Run without any synthetic shadow polyfill patches. - **`FORCE_NATIVE_SHADOW_MODE_FOR_TEST=1`:** Force tests to run in native shadow mode with synthetic shadow polyfill patches. - **`ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE=1`:** Use native custom element lifecycle callbacks. -- **`DISABLE_ARIA_REFLECTION_POLYFILL=1`:** Disable usage of `@lwc/aria-reflection` as a global polyfill. +- **`ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL=1`:** Enable usage of `@lwc/aria-reflection` as a global polyfill. - **`NODE_ENV_FOR_TEST`**: Set the `NODE_ENV` to be used for the tests (at runtime, in the browser). - **`COVERAGE=1`:** Gather engine code coverage, and store it in the `coverage` folder. - **`GREP="pattern"`:** Filter the spec to run based on the pattern. diff --git a/packages/@lwc/integration-karma/scripts/karma-configs/test/tags.js b/packages/@lwc/integration-karma/scripts/karma-configs/test/tags.js index 0bad54f970..7dd342f841 100644 --- a/packages/@lwc/integration-karma/scripts/karma-configs/test/tags.js +++ b/packages/@lwc/integration-karma/scripts/karma-configs/test/tags.js @@ -12,7 +12,7 @@ const { SYNTHETIC_SHADOW_ENABLED, FORCE_NATIVE_SHADOW_MODE_FOR_TEST, ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE, - DISABLE_ARIA_REFLECTION_POLYFILL, + ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL, DISABLE_SYNTHETIC_SHADOW_SUPPORT_IN_COMPILER, NODE_ENV_FOR_TEST, API_VERSION, @@ -24,7 +24,7 @@ const TAGS = [ FORCE_NATIVE_SHADOW_MODE_FOR_TEST && 'force-native-shadow-mode', LEGACY_BROWSERS && 'legacy-browsers', ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE && 'native-lifecycle', - DISABLE_ARIA_REFLECTION_POLYFILL && 'disable-aria-polyfill', + ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL && 'aria-polyfill', DISABLE_SYNTHETIC_SHADOW_SUPPORT_IN_COMPILER && 'disable-synthetic-in-compiler', `NODE_ENV-${NODE_ENV_FOR_TEST}`, API_VERSION && `api-version-${API_VERSION}`, diff --git a/packages/@lwc/integration-karma/scripts/karma-plugins/env.js b/packages/@lwc/integration-karma/scripts/karma-plugins/env.js index 37ea2febf2..04c9a5f648 100644 --- a/packages/@lwc/integration-karma/scripts/karma-plugins/env.js +++ b/packages/@lwc/integration-karma/scripts/karma-plugins/env.js @@ -19,7 +19,7 @@ const { FORCE_NATIVE_SHADOW_MODE_FOR_TEST, SYNTHETIC_SHADOW_ENABLED, ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE, - DISABLE_ARIA_REFLECTION_POLYFILL, + ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL, NODE_ENV_FOR_TEST, API_VERSION, } = require('../shared/options'); @@ -38,7 +38,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}, - DISABLE_ARIA_REFLECTION_POLYFILL: ${DISABLE_ARIA_REFLECTION_POLYFILL} + ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL: ${ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL} }; window.process = { env: { diff --git a/packages/@lwc/integration-karma/scripts/shared/options.js b/packages/@lwc/integration-karma/scripts/shared/options.js index dede4cee9b..c85331aa2b 100644 --- a/packages/@lwc/integration-karma/scripts/shared/options.js +++ b/packages/@lwc/integration-karma/scripts/shared/options.js @@ -18,7 +18,9 @@ const FORCE_NATIVE_SHADOW_MODE_FOR_TEST = Boolean(process.env.FORCE_NATIVE_SHADO const ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE = Boolean( process.env.ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE ); -const DISABLE_ARIA_REFLECTION_POLYFILL = Boolean(process.env.DISABLE_ARIA_REFLECTION_POLYFILL); +const ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL = Boolean( + process.env.ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL +); const DISABLE_SYNTHETIC_SHADOW_SUPPORT_IN_COMPILER = Boolean( process.env.DISABLE_SYNTHETIC_SHADOW_SUPPORT_IN_COMPILER ); @@ -29,7 +31,7 @@ module.exports = { // Test configuration LEGACY_BROWSERS, ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE, - DISABLE_ARIA_REFLECTION_POLYFILL, + ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL, NODE_ENV_FOR_TEST, FORCE_NATIVE_SHADOW_MODE_FOR_TEST, DISABLE_SYNTHETIC_SHADOW_SUPPORT_IN_COMPILER, 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 index fbe8e84f9d..dd8693714b 100644 --- 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.DISABLE_ARIA_REFLECTION_POLYFILL) { +if (window.lwcRuntimeFlags.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 index ed88b866a0..5ab4e6cdd1 100644 --- 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 @@ -37,7 +37,15 @@ if (!process.env.NATIVE_SHADOW) { targetElm = elm.shadowRoot.querySelector('x-aria-target'); }); - [false, true].forEach((usePropertyAccess) => { + const usePropertyAccessValues = [false]; + + // 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) { + usePropertyAccessValues.push(true); + } + + usePropertyAccessValues.forEach((usePropertyAccess) => { const expectedMessages = [ ...(usePropertyAccess ? [expectedMessageForNonStandardAria] : []), expectedMessageForCrossRoot, 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 index 3a83657d00..b8b97b7959 100644 --- 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.DISABLE_ARIA_REFLECTION_POLYFILL) { +if (window.lwcRuntimeFlags.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 index 99b501e4f2..91fa18807c 100644 --- 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.DISABLE_ARIA_REFLECTION_POLYFILL) { + if (window.lwcRuntimeFlags.ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL) { it('aria prop is set', () => { for (const prop of ariaProperties) { expect(childComponent[prop]).toMatch(/^foo/); From 27fe2a46ecce03a8ac05bccae357c64cc5f7706c Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Wed, 9 Aug 2023 09:01:39 -0700 Subject: [PATCH 02/15] chore: clearer name --- .circleci/config.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index c37216570b..490721fd5f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -61,7 +61,7 @@ commands: enable_native_custom_element_lifecycle: type: boolean default: false - enable_aria_reflection_polyfill: + enable_aria_reflection_global_polyfill: type: boolean default: false node_env_production: @@ -95,7 +95,7 @@ commands: <<# parameters.disable_synthetic >> DISABLE_SYNTHETIC=1 <> \ <<# parameters.force_native_shadow_mode >> FORCE_NATIVE_SHADOW_MODE_FOR_TEST=1 <> \ <<# parameters.enable_native_custom_element_lifecycle >> ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE=1 <> \ - <<# parameters.enable_aria_reflection_polyfill >> ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL=1 <> \ + <<# parameters.enable_aria_reflection_global_polyfill >> ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL=1 <> \ <<# parameters.node_env_production >> NODE_ENV_FOR_TEST=production <> \ <<# parameters.disable_synthetic_shadow_support_in_compiler >> DISABLE_SYNTHETIC_SHADOW_SUPPORT_IN_COMPILER=1 <> \ <<# parameters.legacy_browsers >> LEGACY_BROWSERS=1 <> \ @@ -188,7 +188,7 @@ commands: enable_native_custom_element_lifecycle: type: boolean default: false - enable_aria_reflection_polyfill: + enable_aria_reflection_global_polyfill: type: boolean default: false node_env_production: @@ -210,7 +210,7 @@ commands: disable_synthetic: << parameters.disable_synthetic >> force_native_shadow_mode: << parameters.force_native_shadow_mode >> enable_native_custom_element_lifecycle: << parameters.enable_native_custom_element_lifecycle >> - enable_aria_reflection_polyfill: << parameters.enable_aria_reflection_polyfill >> + enable_aria_reflection_global_polyfill: << parameters.enable_aria_reflection_global_polyfill >> node_env_production: << parameters.node_env_production >> disable_synthetic_shadow_support_in_compiler: << parameters.disable_synthetic_shadow_support_in_compiler >> api_version: << parameters.api_version >> @@ -299,10 +299,10 @@ jobs: disable_synthetic: true api_version: 58 - run_karma: - enable_aria_reflection_polyfill: true + enable_aria_reflection_global_polyfill: true - run_karma: disable_synthetic: true - enable_aria_reflection_polyfill: true + enable_aria_reflection_global_polyfill: true - run_karma: disable_synthetic: true disable_synthetic_shadow_support_in_compiler: true From 32c0c7cf6901e12708770b39fcb480940712fcef Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Wed, 9 Aug 2023 09:08:39 -0700 Subject: [PATCH 03/15] fix: update types.ts --- packages/@lwc/features/src/types.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/@lwc/features/src/types.ts b/packages/@lwc/features/src/types.ts index b761710d57..cf5897b4ac 100644 --- a/packages/@lwc/features/src/types.ts +++ b/packages/@lwc/features/src/types.ts @@ -61,9 +61,8 @@ export interface FeatureFlagMap { ENABLE_FROZEN_TEMPLATE: FeatureFlagValue; /** - * Flag to enable the ARIA reflection polyfill. When set to false, this flag will avoid the global DOM patching - * to polyfill ARIA reflection. Instead, the necessary ARIA properties will only exist on the LightningElement - * and HTMLBridgeElement base classes, not on every Element. + * 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; } From d62fe1d87429c82f02b80eb280900f2f14a611ed Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Thu, 10 Aug 2023 09:40:16 -0700 Subject: [PATCH 04/15] chore: empty commit From f5bc9941923bc814ea090260c5df0c5454769904 Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Thu, 10 Aug 2023 12:11:32 -0700 Subject: [PATCH 05/15] chore: bump ci From 4cafff842f6a772ad9f1321e2d41cf65c6f69950 Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Tue, 22 Aug 2023 15:53:46 -0700 Subject: [PATCH 06/15] fix: remove the global polyfill for real --- .../src/patches/detect-non-standard-aria.ts | 21 ++--- .../src/aria-reflection-polyfill.ts | 13 ---- packages/@lwc/engine-dom/src/index.ts | 3 - packages/@lwc/features/src/index.ts | 1 - packages/@lwc/features/src/types.ts | 6 -- .../shared/aria-reflection-global-polyfill.js | 76 +++++++++++++++++++ .../scripts/karma-configs/test/base.js | 20 ++++- .../scripts/karma-plugins/env.js | 4 +- .../karma-plugins/transform-framework.js | 3 + .../non-standard-aria-props/index.spec.js | 2 +- .../synthetic-cross-root-aria/index.spec.js | 2 +- .../polyfills/aria-properties/index.spec.js | 2 +- .../template/attribute-aria/index.spec.js | 2 +- 13 files changed, 115 insertions(+), 40 deletions(-) delete mode 100644 packages/@lwc/engine-dom/src/aria-reflection-polyfill.ts create mode 100644 packages/@lwc/integration-karma/scripts/karma-configs/shared/aria-reflection-global-polyfill.js 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 index 8eddce0402..6f84d7cafc 100644 --- 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 deleted file mode 100644 index 4db542dfa9..0000000000 --- a/packages/@lwc/engine-dom/src/aria-reflection-polyfill.ts +++ /dev/null @@ -1,13 +0,0 @@ -/* - * Copyright (c) 2018, salesforce.com, inc. - * All rights reserved. - * SPDX-License-Identifier: MIT - * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT - */ -import { applyAriaReflection } from '@lwc/aria-reflection'; - -if (lwcRuntimeFlags.ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL) { - // If ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL is true, then we need to apply the ARIA reflection polyfill globally, - // i.e. to the global Element.prototype - applyAriaReflection(); -} diff --git a/packages/@lwc/engine-dom/src/index.ts b/packages/@lwc/engine-dom/src/index.ts index 8a0a54422f..0fdd9c26d4 100644 --- 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 index c0777abdb4..86d0bf5b1f 100644 --- 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 index cf5897b4ac..fd47c3e9d6 100644 --- 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/packages/@lwc/integration-karma/scripts/karma-configs/shared/aria-reflection-global-polyfill.js b/packages/@lwc/integration-karma/scripts/karma-configs/shared/aria-reflection-global-polyfill.js new file mode 100644 index 0000000000..d38e33c14c --- /dev/null +++ b/packages/@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 index ec7bf1f9b9..5ae9ab458c 100644 --- 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 index 04c9a5f648..e5f3866286 100644 --- 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 index 54948cf1fe..cf238ca8c1 100644 --- 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 index dd8693714b..b300972e27 100644 --- 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 index 5ab4e6cdd1..2ae25de457 100644 --- 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 index b8b97b7959..3c66cb7c68 100644 --- 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 index 91fa18807c..a8d338a36a 100644 --- 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/); From 7e5c53a8da8562dd0d884d35142077e4fb468219 Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Tue, 22 Aug 2023 15:55:47 -0700 Subject: [PATCH 07/15] fix: header --- .../karma-configs/shared/aria-reflection-global-polyfill.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/@lwc/integration-karma/scripts/karma-configs/shared/aria-reflection-global-polyfill.js b/packages/@lwc/integration-karma/scripts/karma-configs/shared/aria-reflection-global-polyfill.js index d38e33c14c..6be53c0056 100644 --- a/packages/@lwc/integration-karma/scripts/karma-configs/shared/aria-reflection-global-polyfill.js +++ b/packages/@lwc/integration-karma/scripts/karma-configs/shared/aria-reflection-global-polyfill.js @@ -1,3 +1,9 @@ +/* + * Copyright (c) 2023, salesforce.com, inc. + * All rights reserved. + * SPDX-License-Identifier: MIT + * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT + */ // 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 From 953df88a7c2738755e4c6ce2527f826c971a117d Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Thu, 24 Aug 2023 13:45:53 -0700 Subject: [PATCH 08/15] fix: remove @lwc/aria-reflection entirely --- packages/@lwc/aria-reflection/package.json | 51 ------------------- packages/@lwc/aria-reflection/src/detect.ts | 11 ---- packages/@lwc/aria-reflection/src/index.ts | 20 -------- packages/@lwc/aria-reflection/src/polyfill.ts | 34 ------------- packages/@lwc/aria-reflection/tsconfig.json | 9 ---- packages/@lwc/engine-core/package.json | 1 - .../src/framework/base-bridge-element.ts | 2 +- .../src/framework/base-lightning-element.ts | 2 +- .../src/libs}/aria-reflection/README.md | 18 ------- .../libs/aria-reflection/aria-reflection.ts | 34 +++++++++++++ .../src/patches/detect-non-standard-aria.ts | 8 +-- packages/@lwc/engine-dom/package.json | 1 - packages/@lwc/integration-karma/README.md | 2 +- .../integration-karma/helpers/test-utils.js | 2 +- .../shared/aria-reflection-global-polyfill.js | 2 +- packages/lwc/aria-reflection.d.ts | 7 --- packages/lwc/aria-reflection.js | 7 --- packages/lwc/package.json | 1 - 18 files changed, 43 insertions(+), 169 deletions(-) delete mode 100644 packages/@lwc/aria-reflection/package.json delete mode 100644 packages/@lwc/aria-reflection/src/detect.ts delete mode 100644 packages/@lwc/aria-reflection/src/index.ts delete mode 100644 packages/@lwc/aria-reflection/src/polyfill.ts delete mode 100644 packages/@lwc/aria-reflection/tsconfig.json rename packages/@lwc/{ => engine-core/src/libs}/aria-reflection/README.md (83%) create mode 100644 packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts delete mode 100644 packages/lwc/aria-reflection.d.ts delete mode 100644 packages/lwc/aria-reflection.js diff --git a/packages/@lwc/aria-reflection/package.json b/packages/@lwc/aria-reflection/package.json deleted file mode 100644 index e18a24b5fd..0000000000 --- a/packages/@lwc/aria-reflection/package.json +++ /dev/null @@ -1,51 +0,0 @@ -{ - "//": [ - "THIS FILE IS AUTOGENERATED. If you modify it, it will be rewritten by check-and-rewrite-package-json.js", - "You can safely modify dependencies, devDependencies, keywords, etc., but other props will be overwritten." - ], - "name": "@lwc/aria-reflection", - "version": "3.1.3", - "description": "ARIA element reflection polyfill for strings", - "keywords": [ - "aom", - "aria", - "lwc", - "polyfill", - "reflection" - ], - "homepage": "https://lwc.dev", - "repository": { - "type": "git", - "url": "https://github.com/salesforce/lwc.git", - "directory": "packages/@lwc/aria-reflection" - }, - "bugs": { - "url": "https://github.com/salesforce/lwc/issues" - }, - "license": "MIT", - "publishConfig": { - "access": "public" - }, - "main": "dist/index.cjs.js", - "module": "dist/index.js", - "types": "dist/index.d.ts", - "files": [ - "dist" - ], - "scripts": { - "build": "rollup --config ../../../scripts/rollup/rollup.config.js", - "dev": "rollup --config ../../../scripts/rollup/rollup.config.js --watch --no-watch.clearScreen" - }, - "nx": { - "targets": { - "build": { - "outputs": [ - "{projectRoot}/dist" - ] - } - } - }, - "dependencies": { - "@lwc/shared": "3.1.3" - } -} diff --git a/packages/@lwc/aria-reflection/src/detect.ts b/packages/@lwc/aria-reflection/src/detect.ts deleted file mode 100644 index 0baad671c5..0000000000 --- a/packages/@lwc/aria-reflection/src/detect.ts +++ /dev/null @@ -1,11 +0,0 @@ -/* - * Copyright (c) 2018, salesforce.com, inc. - * All rights reserved. - * SPDX-License-Identifier: MIT - * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT - */ -import { getOwnPropertyDescriptor, isUndefined } from '@lwc/shared'; - -export function detect(propName: string, prototype: any): boolean { - return isUndefined(getOwnPropertyDescriptor(prototype, propName)); -} diff --git a/packages/@lwc/aria-reflection/src/index.ts b/packages/@lwc/aria-reflection/src/index.ts deleted file mode 100644 index b58431d7d6..0000000000 --- a/packages/@lwc/aria-reflection/src/index.ts +++ /dev/null @@ -1,20 +0,0 @@ -/* - * Copyright (c) 2018, salesforce.com, inc. - * All rights reserved. - * SPDX-License-Identifier: MIT - * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT - */ -import { AriaPropNameToAttrNameMap, keys } from '@lwc/shared'; -import { detect } from './detect'; -import { patch } from './polyfill'; - -export function applyAriaReflection(prototype: any = Element.prototype) { - const ElementPrototypeAriaPropertyNames = keys(AriaPropNameToAttrNameMap); - - for (let i = 0, len = ElementPrototypeAriaPropertyNames.length; i < len; i += 1) { - const propName = ElementPrototypeAriaPropertyNames[i]; - if (detect(propName, prototype)) { - patch(propName, prototype); - } - } -} diff --git a/packages/@lwc/aria-reflection/src/polyfill.ts b/packages/@lwc/aria-reflection/src/polyfill.ts deleted file mode 100644 index 9b29dea8a9..0000000000 --- a/packages/@lwc/aria-reflection/src/polyfill.ts +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright (c) 2018, salesforce.com, inc. - * All rights reserved. - * SPDX-License-Identifier: MIT - * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT - */ -import { AriaPropNameToAttrNameMap, isNull, defineProperty } from '@lwc/shared'; - -function createAriaPropertyPropertyDescriptor(attrName: string): PropertyDescriptor { - // Note that we need to call this.{get,set,has,remove}Attribute rather than dereferencing - // from Element.prototype, because these methods are overridden in LightningElement. - return { - get(this: HTMLElement): any { - // reflect what's in the attribute - return this.hasAttribute(attrName) ? this.getAttribute(attrName) : null; - }, - set(this: HTMLElement, newValue: any) { - // reflect into the corresponding attribute - if (isNull(newValue)) { - this.removeAttribute(attrName); - } else { - this.setAttribute(attrName, newValue); - } - }, - configurable: true, - enumerable: true, - }; -} - -export function patch(propName: string, prototype: any) { - const attrName = AriaPropNameToAttrNameMap[propName]; - const descriptor = createAriaPropertyPropertyDescriptor(attrName); - defineProperty(prototype, propName, descriptor); -} diff --git a/packages/@lwc/aria-reflection/tsconfig.json b/packages/@lwc/aria-reflection/tsconfig.json deleted file mode 100644 index 6fa56998a0..0000000000 --- a/packages/@lwc/aria-reflection/tsconfig.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "extends": "../../../tsconfig.json", - - "compilerOptions": { - "lib": ["dom", "es2018"] - }, - - "include": ["src/"] -} diff --git a/packages/@lwc/engine-core/package.json b/packages/@lwc/engine-core/package.json index 9f2cbde5b1..53cf8e2bb9 100644 --- a/packages/@lwc/engine-core/package.json +++ b/packages/@lwc/engine-core/package.json @@ -42,7 +42,6 @@ } }, "dependencies": { - "@lwc/aria-reflection": "3.1.3", "@lwc/features": "3.1.3", "@lwc/shared": "3.1.3" }, diff --git a/packages/@lwc/engine-core/src/framework/base-bridge-element.ts b/packages/@lwc/engine-core/src/framework/base-bridge-element.ts index d1e95a1d97..8b84ac2f2f 100644 --- a/packages/@lwc/engine-core/src/framework/base-bridge-element.ts +++ b/packages/@lwc/engine-core/src/framework/base-bridge-element.ts @@ -20,8 +20,8 @@ import { keys, htmlPropertyToAttribute, } from '@lwc/shared'; -import { applyAriaReflection } from '@lwc/aria-reflection'; import { logError } from '../shared/logger'; +import { applyAriaReflection } from '../libs/aria-reflection/aria-reflection'; import { getAssociatedVM } from './vm'; import { getReadOnlyProxy } from './membrane'; import { HTMLElementConstructor } from './html-element'; diff --git a/packages/@lwc/engine-core/src/framework/base-lightning-element.ts b/packages/@lwc/engine-core/src/framework/base-lightning-element.ts index 08a1e1d4d6..93e231782a 100644 --- a/packages/@lwc/engine-core/src/framework/base-lightning-element.ts +++ b/packages/@lwc/engine-core/src/framework/base-lightning-element.ts @@ -27,10 +27,10 @@ import { keys, setPrototypeOf, } from '@lwc/shared'; -import { applyAriaReflection } from '@lwc/aria-reflection'; import { logError } from '../shared/logger'; import { getComponentTag } from '../shared/format'; +import { applyAriaReflection } from '../libs/aria-reflection/aria-reflection'; import { HTMLElementOriginalDescriptors } from './html-properties'; import { getWrappedComponentsListener } from './component'; diff --git a/packages/@lwc/aria-reflection/README.md b/packages/@lwc/engine-core/src/libs/aria-reflection/README.md similarity index 83% rename from packages/@lwc/aria-reflection/README.md rename to packages/@lwc/engine-core/src/libs/aria-reflection/README.md index c6f9b1d798..b5059a8232 100644 --- a/packages/@lwc/aria-reflection/README.md +++ b/packages/@lwc/engine-core/src/libs/aria-reflection/README.md @@ -1,5 +1,3 @@ -# @lwc/aria-reflection - Polyfill for [ARIA string reflection](https://wicg.github.io/aom/spec/aria-reflection.html) on Elements. This is part of the [Accessibility Object Model](https://wicg.github.io/aom/explainer.html) (AOM). @@ -16,26 +14,10 @@ Note that the attribute `aria-pressed` is reflected to the property `ariaPressed ## Usage -```shell -npm install @lwc/aria-reflection -``` - -```js -import { applyAriaReflection } from '@lwc/aria-reflection'; - -applyAriaReflection(); -``` - -The polyfill is applied as soon as the function is executed. - -Optionally, you can pass in a custom prototype: - ```js applyAriaReflection(MyCustomElement.prototype); ``` -By default, the polyfill is applied to the global `Element.prototype`. - ## Implementation The polyfill patches these [standard](https://w3c.github.io/aria/#idl-interface) properties: diff --git a/packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts b/packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts new file mode 100644 index 0000000000..0fa0005ed1 --- /dev/null +++ b/packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2018, salesforce.com, inc. + * All rights reserved. + * SPDX-License-Identifier: MIT + * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT + */ +import { AriaPropNameToAttrNameMap, keys, isNull, defineProperty } from '@lwc/shared'; + +// Apply ARIA string reflection behavior to a prototype. See README.md for details +export function applyAriaReflection(prototype: any) { + for (const propName of keys(AriaPropNameToAttrNameMap)) { + const attrName = AriaPropNameToAttrNameMap[propName]; + // Note that we need to call this.{get,set,has,remove}Attribute rather than dereferencing + // from Element.prototype, because these methods are overridden in LightningElement. + defineProperty(prototype, propName, { + get(this: HTMLElement): any { + return this.getAttribute(attrName); + }, + set(this: HTMLElement, newValue: any) { + // TODO [#3284]: there is disagreement between browsers and the spec on how to treat undefined + // Our historical behavior is to only treat null as removing the attribute + // See also https://github.com/w3c/aria/issues/1858 + if (isNull(newValue)) { + this.removeAttribute(attrName); + } else { + this.setAttribute(attrName, newValue); + } + }, + // configurable and enumerable to allow it to be overridden – this mimics Safari's/Chrome's behavior + configurable: true, + enumerable: true, + }); + } +} 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 index 6f84d7cafc..a4cadbc2fb 100644 --- a/packages/@lwc/engine-core/src/patches/detect-non-standard-aria.ts +++ b/packages/@lwc/engine-core/src/patches/detect-non-standard-aria.ts @@ -13,10 +13,10 @@ import { BaseBridgeElement } from '../framework/base-bridge-element'; // // The goal of this code is to detect usages of non-standard reflected ARIA properties. These are caused by -// legacy non-standard Element.prototype extensions added by the @lwc/aria-reflection package. +// legacy non-standard Element.prototype extensions added by the aria-reflection.ts. // -// See the README for @lwc/aria-reflection +// See the README for aria-reflection.ts const NON_STANDARD_ARIA_PROPS = [ 'ariaActiveDescendant', 'ariaControls', @@ -85,7 +85,7 @@ function enableDetection() { const { prototype } = Element; for (const prop of NON_STANDARD_ARIA_PROPS) { const descriptor = getOwnPropertyDescriptor(prototype, prop); - // The descriptor should exist because the @lwc/aria-reflection polyfill has run by now. + // The descriptor should exist because the ARIA string reflection polyfill has run by now. // This happens automatically because of the ordering of imports. if (process.env.NODE_ENV !== 'production') { /* istanbul ignore if */ @@ -95,7 +95,7 @@ function enableDetection() { isUndefined(descriptor.set) ) { // should never happen - throw new Error('detect-non-standard-aria.ts loaded before @lwc/aria-reflection'); + throw new Error('detect-non-standard-aria.ts loaded before aria string reflection'); } } // @ts-ignore diff --git a/packages/@lwc/engine-dom/package.json b/packages/@lwc/engine-dom/package.json index 13edef1bc6..2bb7677d39 100644 --- a/packages/@lwc/engine-dom/package.json +++ b/packages/@lwc/engine-dom/package.json @@ -42,7 +42,6 @@ } }, "devDependencies": { - "@lwc/aria-reflection": "3.1.3", "@lwc/engine-core": "3.1.3", "@lwc/shared": "3.1.3", "magic-string": "^0.30.2" diff --git a/packages/@lwc/integration-karma/README.md b/packages/@lwc/integration-karma/README.md index 984a026182..491a3b669a 100644 --- a/packages/@lwc/integration-karma/README.md +++ b/packages/@lwc/integration-karma/README.md @@ -34,7 +34,7 @@ This set of environment variables applies to the `start` and `test` commands: - **`DISABLE_SYNTHETIC=1`:** Run without any synthetic shadow polyfill patches. - **`FORCE_NATIVE_SHADOW_MODE_FOR_TEST=1`:** Force tests to run in native shadow mode with synthetic shadow polyfill patches. - **`ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE=1`:** Use native custom element lifecycle callbacks. -- **`ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL=1`:** Enable usage of `@lwc/aria-reflection` as a global polyfill. +- **`ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL=1`:** ARIA string reflection as a global polyfill. - **`NODE_ENV_FOR_TEST`**: Set the `NODE_ENV` to be used for the tests (at runtime, in the browser). - **`COVERAGE=1`:** Gather engine code coverage, and store it in the `coverage` folder. - **`GREP="pattern"`:** Filter the spec to run based on the pattern. diff --git a/packages/@lwc/integration-karma/helpers/test-utils.js b/packages/@lwc/integration-karma/helpers/test-utils.js index 1a1a0cb469..7c669747b5 100644 --- a/packages/@lwc/integration-karma/helpers/test-utils.js +++ b/packages/@lwc/integration-karma/helpers/test-utils.js @@ -495,7 +495,7 @@ window.TestUtils = (function (lwc, jasmine, beforeAll) { role: 'role', }; - // See the README for @lwc/aria-reflection + // See the README for aria-reflection.ts var nonStandardAriaProperties = [ 'ariaActiveDescendant', 'ariaControls', diff --git a/packages/@lwc/integration-karma/scripts/karma-configs/shared/aria-reflection-global-polyfill.js b/packages/@lwc/integration-karma/scripts/karma-configs/shared/aria-reflection-global-polyfill.js index 6be53c0056..e5d78c2b88 100644 --- a/packages/@lwc/integration-karma/scripts/karma-configs/shared/aria-reflection-global-polyfill.js +++ b/packages/@lwc/integration-karma/scripts/karma-configs/shared/aria-reflection-global-polyfill.js @@ -68,7 +68,7 @@ for (const prop of ARIA_PROPERTIES) { 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 + // aria-reflection.ts's historical behavior. See: https://github.com/w3c/aria/issues/1858 if (value === null) { this.removeAttribute(attribute); } else { diff --git a/packages/lwc/aria-reflection.d.ts b/packages/lwc/aria-reflection.d.ts deleted file mode 100644 index 894c296c68..0000000000 --- a/packages/lwc/aria-reflection.d.ts +++ /dev/null @@ -1,7 +0,0 @@ -/* - * Copyright (c) 2023, salesforce.com, inc. - * All rights reserved. - * SPDX-License-Identifier: MIT - * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT - */ -export type * from '@lwc/aria-reflection'; diff --git a/packages/lwc/aria-reflection.js b/packages/lwc/aria-reflection.js deleted file mode 100644 index b68ed8bec0..0000000000 --- a/packages/lwc/aria-reflection.js +++ /dev/null @@ -1,7 +0,0 @@ -/* - * Copyright (c) 2023, salesforce.com, inc. - * All rights reserved. - * SPDX-License-Identifier: MIT - * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT - */ -export * from '@lwc/aria-reflection'; diff --git a/packages/lwc/package.json b/packages/lwc/package.json index dcd6217a9f..ee24c6440e 100644 --- a/packages/lwc/package.json +++ b/packages/lwc/package.json @@ -20,7 +20,6 @@ "*.d.ts" ], "dependencies": { - "@lwc/aria-reflection": "3.1.3", "@lwc/babel-plugin-component": "3.1.3", "@lwc/compiler": "3.1.3", "@lwc/engine-core": "3.1.3", From 88dd67154a0e15c5355ddf2ca5605c4f4c9b7ed5 Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Thu, 24 Aug 2023 14:51:33 -0700 Subject: [PATCH 09/15] fix: detect before patching --- .../libs/aria-reflection/aria-reflection.ts | 51 +++++++++++-------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts b/packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts index 0fa0005ed1..fc72138f27 100644 --- a/packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts +++ b/packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts @@ -4,31 +4,40 @@ * SPDX-License-Identifier: MIT * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ -import { AriaPropNameToAttrNameMap, keys, isNull, defineProperty } from '@lwc/shared'; +import { + AriaPropNameToAttrNameMap, + defineProperty, + getOwnPropertyDescriptor, + isNull, + isUndefined, + keys, +} from '@lwc/shared'; // Apply ARIA string reflection behavior to a prototype. See README.md for details export function applyAriaReflection(prototype: any) { for (const propName of keys(AriaPropNameToAttrNameMap)) { const attrName = AriaPropNameToAttrNameMap[propName]; - // Note that we need to call this.{get,set,has,remove}Attribute rather than dereferencing - // from Element.prototype, because these methods are overridden in LightningElement. - defineProperty(prototype, propName, { - get(this: HTMLElement): any { - return this.getAttribute(attrName); - }, - set(this: HTMLElement, newValue: any) { - // TODO [#3284]: there is disagreement between browsers and the spec on how to treat undefined - // Our historical behavior is to only treat null as removing the attribute - // See also https://github.com/w3c/aria/issues/1858 - if (isNull(newValue)) { - this.removeAttribute(attrName); - } else { - this.setAttribute(attrName, newValue); - } - }, - // configurable and enumerable to allow it to be overridden – this mimics Safari's/Chrome's behavior - configurable: true, - enumerable: true, - }); + if (isUndefined(getOwnPropertyDescriptor(prototype, propName))) { + // Note that we need to call this.{get,set,has,remove}Attribute rather than dereferencing + // from Element.prototype, because these methods are overridden in LightningElement. + defineProperty(prototype, propName, { + get(this: HTMLElement): any { + return this.getAttribute(attrName); + }, + set(this: HTMLElement, newValue: any) { + // TODO [#3284]: there is disagreement between browsers and the spec on how to treat undefined + // Our historical behavior is to only treat null as removing the attribute + // See also https://github.com/w3c/aria/issues/1858 + if (isNull(newValue)) { + this.removeAttribute(attrName); + } else { + this.setAttribute(attrName, newValue); + } + }, + // configurable and enumerable to allow it to be overridden – this mimics Safari's/Chrome's behavior + configurable: true, + enumerable: true, + }); + } } } From 021998e3e02e758abeada940ec76c6ddd254be4b Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Fri, 25 Aug 2023 12:57:47 -0700 Subject: [PATCH 10/15] fix: retain @lwc/aria-reflection --- .../src/libs => }/aria-reflection/README.md | 13 ++++- packages/@lwc/aria-reflection/package.json | 49 +++++++++++++++++++ .../src/index.ts} | 7 ++- packages/@lwc/aria-reflection/tsconfig.json | 9 ++++ .../libs/aria-reflection/aria-reflection.ts | 7 ++- .../scripts/karma-configs/test/base.js | 13 ++--- .../karma-plugins/transform-framework.js | 2 +- packages/lwc/aria-reflection.d.ts | 7 +++ packages/lwc/aria-reflection.js | 7 +++ packages/lwc/package.json | 1 + 10 files changed, 99 insertions(+), 16 deletions(-) rename packages/@lwc/{engine-core/src/libs => }/aria-reflection/README.md (82%) create mode 100644 packages/@lwc/aria-reflection/package.json rename packages/@lwc/{integration-karma/scripts/karma-configs/shared/aria-reflection-global-polyfill.js => aria-reflection/src/index.ts} (85%) create mode 100644 packages/@lwc/aria-reflection/tsconfig.json create mode 100644 packages/lwc/aria-reflection.d.ts create mode 100644 packages/lwc/aria-reflection.js diff --git a/packages/@lwc/engine-core/src/libs/aria-reflection/README.md b/packages/@lwc/aria-reflection/README.md similarity index 82% rename from packages/@lwc/engine-core/src/libs/aria-reflection/README.md rename to packages/@lwc/aria-reflection/README.md index b5059a8232..36acebb564 100644 --- a/packages/@lwc/engine-core/src/libs/aria-reflection/README.md +++ b/packages/@lwc/aria-reflection/README.md @@ -1,3 +1,8 @@ +# @lwc/aria-reflection + +> **Note:** use this code at your own risk. It is optimized for backwards-compatibility, not +> as a forward-looking polyfill that keeps up to date with web standards. + Polyfill for [ARIA string reflection](https://wicg.github.io/aom/spec/aria-reflection.html) on Elements. This is part of the [Accessibility Object Model](https://wicg.github.io/aom/explainer.html) (AOM). @@ -14,10 +19,16 @@ Note that the attribute `aria-pressed` is reflected to the property `ariaPressed ## Usage +```shell +npm install @lwc/aria-reflection +``` + ```js -applyAriaReflection(MyCustomElement.prototype); +import '@lwc/aria-reflection'; ``` +The polyfill is applied globally to `Element.prototype` as soon as the module is imported. + ## Implementation The polyfill patches these [standard](https://w3c.github.io/aria/#idl-interface) properties: diff --git a/packages/@lwc/aria-reflection/package.json b/packages/@lwc/aria-reflection/package.json new file mode 100644 index 0000000000..eace7b4497 --- /dev/null +++ b/packages/@lwc/aria-reflection/package.json @@ -0,0 +1,49 @@ +{ + "//": [ + "THIS FILE IS AUTOGENERATED. If you modify it, it will be rewritten by check-and-rewrite-package-json.js", + "You can safely modify dependencies, devDependencies, keywords, etc., but other props will be overwritten." + ], + "name": "@lwc/aria-reflection", + "version": "3.2.0", + "description": "ARIA element reflection polyfill for strings", + "keywords": [ + "aom", + "aria", + "lwc", + "polyfill", + "reflection" + ], + "homepage": "https://lwc.dev", + "repository": { + "type": "git", + "url": "https://github.com/salesforce/lwc.git", + "directory": "packages/@lwc/aria-reflection" + }, + "bugs": { + "url": "https://github.com/salesforce/lwc/issues" + }, + "license": "MIT", + "publishConfig": { + "access": "public" + }, + "main": "dist/index.cjs.js", + "module": "dist/index.js", + "types": "dist/index.d.ts", + "files": [ + "dist" + ], + "scripts": { + "build": "rollup --config ../../../scripts/rollup/rollup.config.js", + "dev": "rollup --config ../../../scripts/rollup/rollup.config.js --watch --no-watch.clearScreen" + }, + "nx": { + "targets": { + "build": { + "outputs": [ + "{projectRoot}/dist" + ] + } + } + }, + "dependencies": {} +} diff --git a/packages/@lwc/integration-karma/scripts/karma-configs/shared/aria-reflection-global-polyfill.js b/packages/@lwc/aria-reflection/src/index.ts similarity index 85% rename from packages/@lwc/integration-karma/scripts/karma-configs/shared/aria-reflection-global-polyfill.js rename to packages/@lwc/aria-reflection/src/index.ts index e5d78c2b88..efad8f79d3 100644 --- a/packages/@lwc/integration-karma/scripts/karma-configs/shared/aria-reflection-global-polyfill.js +++ b/packages/@lwc/aria-reflection/src/index.ts @@ -4,9 +4,11 @@ * SPDX-License-Identifier: MIT * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ + // 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 +// This is designed for maximum backwards compatibility on LEX - it should never change. +// We deliberately don't import from @lwc/shared because that would make this code less portable. const ARIA_PROPERTIES = [ 'ariaActiveDescendant', 'ariaAtomic', @@ -68,13 +70,14 @@ for (const prop of ARIA_PROPERTIES) { 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 - // aria-reflection.ts's historical behavior. See: https://github.com/w3c/aria/issues/1858 + // our historical behavior. See: https://github.com/w3c/aria/issues/1858 if (value === null) { this.removeAttribute(attribute); } else { this.setAttribute(attribute, value); } }, + // configurable and enumerable to allow it to be overridden – this mimics Safari's/Chrome's behavior configurable: true, enumerable: true, }); diff --git a/packages/@lwc/aria-reflection/tsconfig.json b/packages/@lwc/aria-reflection/tsconfig.json new file mode 100644 index 0000000000..6fa56998a0 --- /dev/null +++ b/packages/@lwc/aria-reflection/tsconfig.json @@ -0,0 +1,9 @@ +{ + "extends": "../../../tsconfig.json", + + "compilerOptions": { + "lib": ["dom", "es2018"] + }, + + "include": ["src/"] +} diff --git a/packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts b/packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts index fc72138f27..6e5ca4dc3c 100644 --- a/packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts +++ b/packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018, salesforce.com, inc. + * Copyright (c) 2023, salesforce.com, inc. * All rights reserved. * SPDX-License-Identifier: MIT * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT @@ -13,7 +13,10 @@ import { keys, } from '@lwc/shared'; -// Apply ARIA string reflection behavior to a prototype. See README.md for details +// Apply ARIA string reflection behavior to a prototype. +// This is deliberately kept separate from @lwc/aria-reflection. @lwc/aria-reflection is a global polyfill that is +// needed for backwards compatibility in LEX, whereas `applyAriaReflection` is designed to only apply to our own +// LightningElement/BaseBridgeElement prototypes. export function applyAriaReflection(prototype: any) { for (const propName of keys(AriaPropNameToAttrNameMap)) { const attrName = AriaPropNameToAttrNameMap[propName]; diff --git a/packages/@lwc/integration-karma/scripts/karma-configs/test/base.js b/packages/@lwc/integration-karma/scripts/karma-configs/test/base.js index 5ae9ab458c..50bc686601 100644 --- a/packages/@lwc/integration-karma/scripts/karma-configs/test/base.js +++ b/packages/@lwc/integration-karma/scripts/karma-configs/test/base.js @@ -27,20 +27,13 @@ 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 ARIA_REFLECTION = require.resolve('@lwc/aria-reflection/dist/index.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, - ARIA_REFLECTION_GLOBAL_POLYFILL, -]; +const ALL_FRAMEWORK_FILES = [SYNTHETIC_SHADOW, LWC_ENGINE, WIRE_SERVICE, ARIA_REFLECTION]; // 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 @@ -55,7 +48,7 @@ function getFiles() { frameworkFiles.push(createPattern(SYNTHETIC_SHADOW)); } if (ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL) { - frameworkFiles.push(createPattern(ARIA_REFLECTION_GLOBAL_POLYFILL)); + frameworkFiles.push(createPattern(ARIA_REFLECTION)); } frameworkFiles.push(createPattern(LWC_ENGINE)); frameworkFiles.push(createPattern(WIRE_SERVICE)); diff --git a/packages/@lwc/integration-karma/scripts/karma-plugins/transform-framework.js b/packages/@lwc/integration-karma/scripts/karma-plugins/transform-framework.js index cf238ca8c1..68e3dea130 100644 --- a/packages/@lwc/integration-karma/scripts/karma-plugins/transform-framework.js +++ b/packages/@lwc/integration-karma/scripts/karma-plugins/transform-framework.js @@ -24,7 +24,7 @@ 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')) { + } else if (filename.includes('aria-reflection')) { // aria reflection global polyfill does not need an IIFE name return undefined; } diff --git a/packages/lwc/aria-reflection.d.ts b/packages/lwc/aria-reflection.d.ts new file mode 100644 index 0000000000..894c296c68 --- /dev/null +++ b/packages/lwc/aria-reflection.d.ts @@ -0,0 +1,7 @@ +/* + * Copyright (c) 2023, salesforce.com, inc. + * All rights reserved. + * SPDX-License-Identifier: MIT + * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT + */ +export type * from '@lwc/aria-reflection'; diff --git a/packages/lwc/aria-reflection.js b/packages/lwc/aria-reflection.js new file mode 100644 index 0000000000..b68ed8bec0 --- /dev/null +++ b/packages/lwc/aria-reflection.js @@ -0,0 +1,7 @@ +/* + * Copyright (c) 2023, salesforce.com, inc. + * All rights reserved. + * SPDX-License-Identifier: MIT + * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT + */ +export * from '@lwc/aria-reflection'; diff --git a/packages/lwc/package.json b/packages/lwc/package.json index af6a819935..2dab2f7ecc 100644 --- a/packages/lwc/package.json +++ b/packages/lwc/package.json @@ -20,6 +20,7 @@ "*.d.ts" ], "dependencies": { + "@lwc/aria-reflection": "3.2.0", "@lwc/babel-plugin-component": "3.2.0", "@lwc/compiler": "3.2.0", "@lwc/engine-core": "3.2.0", From e7949e6bb9feb8834cb8784bb680ab977433b257 Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Fri, 25 Aug 2023 13:01:41 -0700 Subject: [PATCH 11/15] fix: fix comments --- .../engine-core/src/patches/detect-non-standard-aria.ts | 8 ++++---- packages/@lwc/integration-karma/helpers/test-utils.js | 2 +- 2 files changed, 5 insertions(+), 5 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 index a4cadbc2fb..6f84d7cafc 100644 --- a/packages/@lwc/engine-core/src/patches/detect-non-standard-aria.ts +++ b/packages/@lwc/engine-core/src/patches/detect-non-standard-aria.ts @@ -13,10 +13,10 @@ import { BaseBridgeElement } from '../framework/base-bridge-element'; // // The goal of this code is to detect usages of non-standard reflected ARIA properties. These are caused by -// legacy non-standard Element.prototype extensions added by the aria-reflection.ts. +// legacy non-standard Element.prototype extensions added by the @lwc/aria-reflection package. // -// See the README for aria-reflection.ts +// See the README for @lwc/aria-reflection const NON_STANDARD_ARIA_PROPS = [ 'ariaActiveDescendant', 'ariaControls', @@ -85,7 +85,7 @@ function enableDetection() { const { prototype } = Element; for (const prop of NON_STANDARD_ARIA_PROPS) { const descriptor = getOwnPropertyDescriptor(prototype, prop); - // The descriptor should exist because the ARIA string reflection polyfill has run by now. + // The descriptor should exist because the @lwc/aria-reflection polyfill has run by now. // This happens automatically because of the ordering of imports. if (process.env.NODE_ENV !== 'production') { /* istanbul ignore if */ @@ -95,7 +95,7 @@ function enableDetection() { isUndefined(descriptor.set) ) { // should never happen - throw new Error('detect-non-standard-aria.ts loaded before aria string reflection'); + throw new Error('detect-non-standard-aria.ts loaded before @lwc/aria-reflection'); } } // @ts-ignore diff --git a/packages/@lwc/integration-karma/helpers/test-utils.js b/packages/@lwc/integration-karma/helpers/test-utils.js index 7c669747b5..1a1a0cb469 100644 --- a/packages/@lwc/integration-karma/helpers/test-utils.js +++ b/packages/@lwc/integration-karma/helpers/test-utils.js @@ -495,7 +495,7 @@ window.TestUtils = (function (lwc, jasmine, beforeAll) { role: 'role', }; - // See the README for aria-reflection.ts + // See the README for @lwc/aria-reflection var nonStandardAriaProperties = [ 'ariaActiveDescendant', 'ariaControls', From 540eeb7e701d9ecbe1a6fd64ee34f6eb8c2e0894 Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Fri, 1 Sep 2023 09:16:04 -0700 Subject: [PATCH 12/15] fix: tighten prototype types --- .../engine-core/src/libs/aria-reflection/aria-reflection.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts b/packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts index 6e5ca4dc3c..6df29c5026 100644 --- a/packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts +++ b/packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts @@ -12,12 +12,13 @@ import { isUndefined, keys, } from '@lwc/shared'; +import { LightningElement } from '../../framework/base-lightning-element'; // Apply ARIA string reflection behavior to a prototype. // This is deliberately kept separate from @lwc/aria-reflection. @lwc/aria-reflection is a global polyfill that is // needed for backwards compatibility in LEX, whereas `applyAriaReflection` is designed to only apply to our own // LightningElement/BaseBridgeElement prototypes. -export function applyAriaReflection(prototype: any) { +export function applyAriaReflection(prototype: HTMLElement | LightningElement) { for (const propName of keys(AriaPropNameToAttrNameMap)) { const attrName = AriaPropNameToAttrNameMap[propName]; if (isUndefined(getOwnPropertyDescriptor(prototype, propName))) { From 4cf39a81fa8c734c970fb8860800698afb5baa18 Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Fri, 1 Sep 2023 09:19:32 -0700 Subject: [PATCH 13/15] fix: comment --- .../@lwc/engine-core/src/patches/detect-non-standard-aria.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 index 6f84d7cafc..768f6841d4 100644 --- a/packages/@lwc/engine-core/src/patches/detect-non-standard-aria.ts +++ b/packages/@lwc/engine-core/src/patches/detect-non-standard-aria.ts @@ -29,7 +29,9 @@ const NON_STANDARD_ARIA_PROPS = [ ]; function isGlobalAriaPolyfillLoaded(): boolean { - // sniff for the legacy polyfill being loaded + // Sniff for the legacy polyfill being loaded. The reason this works is because ariaActiveDescendant is a + // non-standard ARIA property reflection that is only supported in our legacy polyfill. See + // @lwc/aria-reflection/README.md for details. return !isUndefined(getOwnPropertyDescriptor(Element.prototype, 'ariaActiveDescendant')); } From 892212bb2c879af10236d88f14731deac42691f4 Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Fri, 1 Sep 2023 09:39:28 -0700 Subject: [PATCH 14/15] test: add test for non-standard props NOT reporting --- .../polyfills/aria-properties/index.spec.js | 40 ++++++++++++++++++- .../x/component/component.html | 1 + .../aria-properties/x/component/component.js | 8 ++++ 3 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 packages/@lwc/integration-karma/test/polyfills/aria-properties/x/component/component.html create mode 100644 packages/@lwc/integration-karma/test/polyfills/aria-properties/x/component/component.js 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 index 3c66cb7c68..7a9524dcc2 100644 --- a/packages/@lwc/integration-karma/test/polyfills/aria-properties/index.spec.js +++ b/packages/@lwc/integration-karma/test/polyfills/aria-properties/index.spec.js @@ -1,5 +1,6 @@ import { ariaPropertiesMapping, nonStandardAriaProperties } from 'test-utils'; -import { __unstable__ReportingControl as reportingControl } from 'lwc'; +import { __unstable__ReportingControl as reportingControl, createElement } from 'lwc'; +import Component from 'x/component'; function testAriaProperty(property, attribute) { describe(property, () => { @@ -170,4 +171,41 @@ if (process.env.ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL) { for (const [ariaProperty, ariaAttribute] of Object.entries(ariaPropertiesMapping)) { testAriaProperty(ariaProperty, ariaAttribute); } + + describe('non-standard properties do not log/report for LightningElement/BaseBridgeElement', () => { + let dispatcher; + + beforeEach(() => { + dispatcher = jasmine.createSpy(); + reportingControl.attachDispatcher(dispatcher); + }); + + afterEach(() => { + reportingControl.detachDispatcher(); + }); + + nonStandardAriaProperties.forEach((prop) => { + describe(prop, () => { + it('LightningElement (internal)', () => { + const elm = createElement('x-component', { is: Component }); + document.body.appendChild(elm); + + expect(() => { + elm.getProp(prop); + }).not.toLogWarningDev(); + expect(dispatcher).not.toHaveBeenCalled(); + }); + + it('BaseBridgeElement (external)', () => { + const elm = createElement('x-component', { is: Component }); + document.body.appendChild(elm); + + expect(() => { + elm[prop] = 'foo'; + }).not.toLogWarningDev(); + expect(dispatcher).not.toHaveBeenCalled(); + }); + }); + }); + }); } diff --git a/packages/@lwc/integration-karma/test/polyfills/aria-properties/x/component/component.html b/packages/@lwc/integration-karma/test/polyfills/aria-properties/x/component/component.html new file mode 100644 index 0000000000..cc340bc4c9 --- /dev/null +++ b/packages/@lwc/integration-karma/test/polyfills/aria-properties/x/component/component.html @@ -0,0 +1 @@ + diff --git a/packages/@lwc/integration-karma/test/polyfills/aria-properties/x/component/component.js b/packages/@lwc/integration-karma/test/polyfills/aria-properties/x/component/component.js new file mode 100644 index 0000000000..a9afef90cd --- /dev/null +++ b/packages/@lwc/integration-karma/test/polyfills/aria-properties/x/component/component.js @@ -0,0 +1,8 @@ +import { LightningElement, api } from 'lwc'; + +export default class extends LightningElement { + @api + getProp(name) { + return this[name]; + } +} From 8ab13faa9799cbc499a76b4692e3922cf48ccac1 Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Fri, 1 Sep 2023 09:46:27 -0700 Subject: [PATCH 15/15] fix: add explanatory comment --- .../@lwc/engine-core/src/patches/detect-non-standard-aria.ts | 3 +++ 1 file changed, 3 insertions(+) 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 index 768f6841d4..38567d6d23 100644 --- a/packages/@lwc/engine-core/src/patches/detect-non-standard-aria.ts +++ b/packages/@lwc/engine-core/src/patches/detect-non-standard-aria.ts @@ -102,6 +102,9 @@ function enableDetection() { } // @ts-ignore const { get, set } = descriptor; + // It's important for this defineProperty call to happen _after_ ARIA accessors are applied to the + // BaseBridgeElement and LightningElement prototypes. Otherwise, we will log/report for access of non-standard + // props on these prototypes, which we actually don't want. We only care about access on generic HTMLElements. defineProperty(prototype, prop, { get() { checkAndReportViolation(this, prop, false, undefined);