diff --git a/.circleci/config.yml b/.circleci/config.yml index 2d002b2ff5..09a50e9461 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_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.disable_aria_reflection_polyfill >> DISABLE_ARIA_REFLECTION_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 - disable_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 >> - disable_aria_reflection_polyfill: << parameters.disable_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 >> @@ -303,9 +303,11 @@ jobs: - run_karma: disable_synthetic: true api_version: 59 + - run_karma: + enable_aria_reflection_global_polyfill: true - run_karma: disable_synthetic: true - disable_aria_reflection_polyfill: true + enable_aria_reflection_global_polyfill: true - run_karma: disable_synthetic: true disable_synthetic_shadow_support_in_compiler: true diff --git a/packages/@lwc/aria-reflection/README.md b/packages/@lwc/aria-reflection/README.md index c6f9b1d798..36acebb564 100644 --- a/packages/@lwc/aria-reflection/README.md +++ b/packages/@lwc/aria-reflection/README.md @@ -1,5 +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). @@ -21,20 +24,10 @@ 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); +import '@lwc/aria-reflection'; ``` -By default, the polyfill is applied to the global `Element.prototype`. +The polyfill is applied globally to `Element.prototype` as soon as the module is imported. ## Implementation diff --git a/packages/@lwc/aria-reflection/package.json b/packages/@lwc/aria-reflection/package.json index 65ef164c88..8ae1d4ab92 100644 --- a/packages/@lwc/aria-reflection/package.json +++ b/packages/@lwc/aria-reflection/package.json @@ -45,7 +45,5 @@ } } }, - "dependencies": { - "@lwc/shared": "3.8.0" - } + "dependencies": {} } 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 index b58431d7d6..efad8f79d3 100644 --- a/packages/@lwc/aria-reflection/src/index.ts +++ b/packages/@lwc/aria-reflection/src/index.ts @@ -1,20 +1,85 @@ /* - * 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 */ -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); +// 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 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', + '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 (let i = 0, len = ElementPrototypeAriaPropertyNames.length; i < len; i += 1) { - const propName = ElementPrototypeAriaPropertyNames[i]; - if (detect(propName, prototype)) { - patch(propName, prototype); - } +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 + // 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/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/engine-core/package.json b/packages/@lwc/engine-core/package.json index 023faa4657..eda29562d4 100644 --- a/packages/@lwc/engine-core/package.json +++ b/packages/@lwc/engine-core/package.json @@ -42,7 +42,6 @@ } }, "dependencies": { - "@lwc/aria-reflection": "3.8.0", "@lwc/features": "3.8.0", "@lwc/shared": "3.8.0" }, 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 c52a869b6e..fb64a6261f 100644 --- a/packages/@lwc/engine-core/src/framework/base-bridge-element.ts +++ b/packages/@lwc/engine-core/src/framework/base-bridge-element.ts @@ -23,7 +23,7 @@ import { htmlPropertyToAttribute, isNull, } from '@lwc/shared'; -import { applyAriaReflection } from '@lwc/aria-reflection'; +import { applyAriaReflection } from '../libs/aria-reflection/aria-reflection'; import { logWarn } from '../shared/logger'; import { getAssociatedVM } from './vm'; import { getReadOnlyProxy } from './membrane'; @@ -263,15 +263,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 0558e4de05..8c68712b6d 100644 --- a/packages/@lwc/engine-core/src/framework/base-lightning-element.ts +++ b/packages/@lwc/engine-core/src/framework/base-lightning-element.ts @@ -28,10 +28,10 @@ import { keys, setPrototypeOf, } from '@lwc/shared'; -import { applyAriaReflection } from '@lwc/aria-reflection'; import { logError, logWarn } 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'; @@ -816,16 +816,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/libs/aria-reflection/aria-reflection.ts b/packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts new file mode 100644 index 0000000000..6df29c5026 --- /dev/null +++ b/packages/@lwc/engine-core/src/libs/aria-reflection/aria-reflection.ts @@ -0,0 +1,47 @@ +/* + * 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 + */ +import { + AriaPropNameToAttrNameMap, + defineProperty, + getOwnPropertyDescriptor, + isNull, + 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: HTMLElement | LightningElement) { + for (const propName of keys(AriaPropNameToAttrNameMap)) { + const attrName = AriaPropNameToAttrNameMap[propName]; + 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, + }); + } + } +} 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..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 @@ -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,10 +28,11 @@ 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 isGlobalAriaPolyfillLoaded(): boolean { + // 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')); } function findVM(elm: Element): VM | undefined { @@ -45,7 +45,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 +57,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() { @@ -103,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); @@ -119,14 +121,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.DISABLE_ARIA_REFLECTION_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/package.json b/packages/@lwc/engine-dom/package.json index 404b25ae33..1935062349 100644 --- a/packages/@lwc/engine-dom/package.json +++ b/packages/@lwc/engine-dom/package.json @@ -42,7 +42,6 @@ } }, "devDependencies": { - "@lwc/aria-reflection": "3.8.0", "@lwc/engine-core": "3.8.0", "@lwc/shared": "3.8.0" }, 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 b3aebab5fa..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.DISABLE_ARIA_REFLECTION_POLYFILL) { - // If DISABLE_ARIA_REFLECTION_POLYFILL is false, 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 de409ce235..bc3b9fcab6 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/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 1ed2dc5735..f362292df6 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, - DISABLE_ARIA_REFLECTION_POLYFILL: null, ENABLE_LEGACY_SCOPE_TOKENS: null, }; diff --git a/packages/@lwc/features/src/types.ts b/packages/@lwc/features/src/types.ts index 3779c1ab6a..46185538ec 100644 --- a/packages/@lwc/features/src/types.ts +++ b/packages/@lwc/features/src/types.ts @@ -60,13 +60,6 @@ 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 - * 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; - /** * If true, render legacy CSS scope tokens in addition to the modern CSS scope tokens. This is designed * for cases where backwards compat is required (e.g. global stylesheets using these tokens in their selectors). diff --git a/packages/@lwc/integration-karma/README.md b/packages/@lwc/integration-karma/README.md index f637f65def..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. -- **`DISABLE_ARIA_REFLECTION_POLYFILL=1`:** Disable 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/scripts/karma-configs/test/base.js b/packages/@lwc/integration-karma/scripts/karma-configs/test/base.js index d2782749c1..5f437ad310 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,11 +27,12 @@ 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 = require.resolve('@lwc/aria-reflection/dist/index.js'); const TEST_UTILS = require.resolve('../../../helpers/test-utils'); 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]; // 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 @@ -40,6 +46,9 @@ function getFiles() { if (SYNTHETIC_SHADOW_ENABLED) { frameworkFiles.push(createPattern(SYNTHETIC_SHADOW)); } + if (ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL) { + frameworkFiles.push(createPattern(ARIA_REFLECTION)); + } frameworkFiles.push(createPattern(LWC_ENGINE)); frameworkFiles.push(createPattern(WIRE_SERVICE)); frameworkFiles.push(createPattern(TEST_SETUP)); 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..e5f3866286 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'); @@ -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}, - DISABLE_ARIA_REFLECTION_POLYFILL: ${DISABLE_ARIA_REFLECTION_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..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,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')) { + // 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/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..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.DISABLE_ARIA_REFLECTION_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 ed88b866a0..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 @@ -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 (process.env.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 ae5c5f10e2..3f168ab557 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, () => { @@ -189,8 +190,45 @@ 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 (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]; + } +} 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..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.DISABLE_ARIA_REFLECTION_POLYFILL) { + if (process.env.ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL) { it('aria prop is set', () => { for (const prop of ariaProperties) { expect(childComponent[prop]).toMatch(/^foo/);