Skip to content

Commit

Permalink
fix(engine-core): unrevert ARIA reflection fix and release 5.0.7 (#3937)
Browse files Browse the repository at this point in the history
Reverts a7e0b54 effectively.
  • Loading branch information
nolanlawson committed Jan 10, 2024
1 parent ab701e5 commit 23ee7fd
Show file tree
Hide file tree
Showing 38 changed files with 882 additions and 131 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "lwc-monorepo",
"version": "5.0.6",
"version": "5.0.7",
"private": true,
"description": "Lightning Web Components",
"repository": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@lwc/aria-reflection/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"You can safely modify dependencies, devDependencies, keywords, etc., but other props will be overwritten."
],
"name": "@lwc/aria-reflection",
"version": "5.0.6",
"version": "5.0.7",
"description": "ARIA element reflection polyfill for strings",
"keywords": [
"aom",
Expand Down
6 changes: 3 additions & 3 deletions packages/@lwc/babel-plugin-component/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"You can safely modify dependencies, devDependencies, keywords, etc., but other props will be overwritten."
],
"name": "@lwc/babel-plugin-component",
"version": "5.0.6",
"version": "5.0.7",
"description": "Babel plugin to transform a LWC module",
"keywords": [
"lwc"
Expand Down Expand Up @@ -43,8 +43,8 @@
},
"dependencies": {
"@babel/helper-module-imports": "7.22.15",
"@lwc/errors": "5.0.6",
"@lwc/shared": "5.0.6",
"@lwc/errors": "5.0.7",
"@lwc/shared": "5.0.7",
"line-column": "~1.0.2"
},
"devDependencies": {
Expand Down
12 changes: 6 additions & 6 deletions packages/@lwc/compiler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"You can safely modify dependencies, devDependencies, keywords, etc., but other props will be overwritten."
],
"name": "@lwc/compiler",
"version": "5.0.6",
"version": "5.0.7",
"description": "LWC compiler",
"keywords": [
"lwc"
Expand Down Expand Up @@ -48,10 +48,10 @@
"@babel/plugin-proposal-object-rest-spread": "7.20.7",
"@babel/plugin-transform-async-to-generator": "7.23.3",
"@locker/babel-plugin-transform-unforgeables": "0.20.0",
"@lwc/babel-plugin-component": "5.0.6",
"@lwc/errors": "5.0.6",
"@lwc/shared": "5.0.6",
"@lwc/style-compiler": "5.0.6",
"@lwc/template-compiler": "5.0.6"
"@lwc/babel-plugin-component": "5.0.7",
"@lwc/errors": "5.0.7",
"@lwc/shared": "5.0.7",
"@lwc/style-compiler": "5.0.7",
"@lwc/template-compiler": "5.0.7"
}
}
6 changes: 3 additions & 3 deletions packages/@lwc/engine-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"You can safely modify dependencies, devDependencies, keywords, etc., but other props will be overwritten."
],
"name": "@lwc/engine-core",
"version": "5.0.6",
"version": "5.0.7",
"description": "Core LWC engine APIs.",
"keywords": [
"lwc"
Expand Down Expand Up @@ -42,8 +42,8 @@
}
},
"dependencies": {
"@lwc/features": "5.0.6",
"@lwc/shared": "5.0.6"
"@lwc/features": "5.0.7",
"@lwc/shared": "5.0.7"
},
"devDependencies": {
"observable-membrane": "2.0.0"
Expand Down
41 changes: 23 additions & 18 deletions packages/@lwc/engine-core/src/framework/base-bridge-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
htmlPropertyToAttribute,
isNull,
} from '@lwc/shared';
import { applyAriaReflection } from '../libs/aria-reflection/aria-reflection';
import { ariaReflectionPolyfillDescriptors } from '../libs/aria-reflection/aria-reflection';
import { logWarn } from '../shared/logger';
import { getAssociatedVM } from './vm';
import { getReadOnlyProxy } from './membrane';
Expand Down Expand Up @@ -164,7 +164,11 @@ export function HTMLBridgeElementFactory(
// and can break tooling that expects it to be iterable or defined, e.g. Jest:
// https://github.com/jestjs/jest/blob/b4c9587/packages/pretty-format/src/plugins/DOMElement.ts#L95
// It also doesn't make sense to override e.g. "constructor".
.filter((propName) => !(propName in HTMLElementPrototype))
.filter(
(propName) =>
!(propName in HTMLElementPrototype) &&
!(propName in ariaReflectionPolyfillDescriptors)
)
);

for (const propName of nonPublicPropertiesToWarnOn) {
Expand Down Expand Up @@ -250,29 +254,30 @@ export function HTMLBridgeElementFactory(
return HTMLBridgeElement as HTMLElementConstructor;
}

// We do some special handling of non-standard ARIA props like ariaLabelledBy as well as props without (as of this
// writing) broad cross-browser support like ariaBrailleLabel. This is so the reflection works correctly and preserves
// backwards compatibility with the previous global polyfill approach.
//
// The goal here is to expose `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 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
const basePublicProperties = [
...getOwnPropertyNames(HTMLElementOriginalDescriptors),
...(process.env.IS_BROWSER ? getOwnPropertyNames(ariaReflectionPolyfillDescriptors) : []),
];

export const BaseBridgeElement = HTMLBridgeElementFactory(
HTMLElementConstructor,
getOwnPropertyNames(HTMLElementOriginalDescriptors),
basePublicProperties,
[],
[],
null,
false
);

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
//
// 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);
seal(BaseBridgeElement.prototype);
22 changes: 18 additions & 4 deletions packages/@lwc/engine-core/src/framework/base-lightning-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
create,
defineProperties,
defineProperty,
entries,
freeze,
hasOwnProperty,
isFunction,
Expand All @@ -31,7 +32,7 @@ import {

import { logError, logWarn } from '../shared/logger';
import { getComponentTag } from '../shared/format';
import { applyAriaReflection } from '../libs/aria-reflection/aria-reflection';
import { ariaReflectionPolyfillDescriptors } from '../libs/aria-reflection/aria-reflection';

import { HTMLElementOriginalDescriptors } from './html-properties';
import { getWrappedComponentsListener } from './component';
Expand Down Expand Up @@ -814,12 +815,25 @@ for (const propName in HTMLElementOriginalDescriptors) {
);
}

defineProperties(LightningElement.prototype, lightningBasedDescriptors);

// 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);
if (process.env.IS_BROWSER) {
// In the browser, we use createBridgeToElementDescriptor, so we can get the normal reactivity lifecycle for
// aria* properties
for (const [propName, descriptor] of entries(ariaReflectionPolyfillDescriptors) as [
name: string,
descriptor: PropertyDescriptor
][]) {
lightningBasedDescriptors[propName] = createBridgeToElementDescriptor(propName, descriptor);
}
} else {
// On the server, we cannot use createBridgeToElementDescriptor because getAttribute/setAttribute are
// not supported on HTMLElement. So apply the polyfill directly on top of LightningElement
defineProperties(LightningElement.prototype, ariaReflectionPolyfillDescriptors);
}

defineProperties(LightningElement.prototype, lightningBasedDescriptors);

defineProperty(LightningElement, 'CustomElementConstructor', {
get() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,42 +6,41 @@
*/
import {
AriaPropNameToAttrNameMap,
defineProperty,
getOwnPropertyDescriptor,
isNull,
isUndefined,
keys,
create,
getPropertyDescriptor,
entries,
} from '@lwc/shared';
import { LightningElement } from '../../framework/base-lightning-element';
import { HTMLElementPrototype } from '../../framework/html-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
// needed for backwards compatibility in LEX, whereas this 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,
});
}
// Note we only need to handle ARIA reflections that aren't already in Element.prototype
export const ariaReflectionPolyfillDescriptors = create(null);
for (const [propName, attrName] of entries(AriaPropNameToAttrNameMap)) {
if (isUndefined(getPropertyDescriptor(HTMLElementPrototype, 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.
ariaReflectionPolyfillDescriptors[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,
};
}
}
6 changes: 3 additions & 3 deletions packages/@lwc/engine-dom/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"You can safely modify dependencies, devDependencies, keywords, etc., but other props will be overwritten."
],
"name": "@lwc/engine-dom",
"version": "5.0.6",
"version": "5.0.7",
"description": "Renders LWC components in a DOM environment.",
"keywords": [
"lwc"
Expand Down Expand Up @@ -42,8 +42,8 @@
}
},
"devDependencies": {
"@lwc/engine-core": "5.0.6",
"@lwc/shared": "5.0.6"
"@lwc/engine-core": "5.0.7",
"@lwc/shared": "5.0.7"
},
"lwc": {
"modules": [
Expand Down
8 changes: 4 additions & 4 deletions packages/@lwc/engine-server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"You can safely modify dependencies, devDependencies, keywords, etc., but other props will be overwritten."
],
"name": "@lwc/engine-server",
"version": "5.0.6",
"version": "5.0.7",
"description": "Renders LWC components in a server environment.",
"keywords": [
"lwc"
Expand Down Expand Up @@ -42,9 +42,9 @@
}
},
"devDependencies": {
"@lwc/engine-core": "5.0.6",
"@lwc/rollup-plugin": "5.0.6",
"@lwc/shared": "5.0.6",
"@lwc/engine-core": "5.0.7",
"@lwc/rollup-plugin": "5.0.7",
"@lwc/shared": "5.0.7",
"@rollup/plugin-virtual": "^3.0.1",
"parse5": "^7.1.2",
"@parse5/tools": "^0.3.0"
Expand Down
2 changes: 1 addition & 1 deletion packages/@lwc/errors/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"You can safely modify dependencies, devDependencies, keywords, etc., but other props will be overwritten."
],
"name": "@lwc/errors",
"version": "5.0.6",
"version": "5.0.7",
"description": "LWC Error Utilities",
"keywords": [
"lwc"
Expand Down
4 changes: 2 additions & 2 deletions packages/@lwc/features/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"You can safely modify dependencies, devDependencies, keywords, etc., but other props will be overwritten."
],
"name": "@lwc/features",
"version": "5.0.6",
"version": "5.0.7",
"description": "LWC Features Flags",
"keywords": [
"lwc"
Expand Down Expand Up @@ -42,6 +42,6 @@
}
},
"dependencies": {
"@lwc/shared": "5.0.6"
"@lwc/shared": "5.0.7"
}
}
12 changes: 6 additions & 6 deletions packages/@lwc/integration-karma/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@lwc/integration-karma",
"private": true,
"version": "5.0.6",
"version": "5.0.7",
"scripts": {
"start": "KARMA_MODE=watch karma start ./scripts/karma-configs/test/local.js",
"test": "karma start ./scripts/karma-configs/test/local.js --single-run --browsers ChromeHeadless",
Expand All @@ -18,11 +18,11 @@
"karma-sauce-launcher-fix-firefox": "using a fork to work around https://github.com/karma-runner/karma-sauce-launcher/issues/275"
},
"devDependencies": {
"@lwc/compiler": "5.0.6",
"@lwc/engine-dom": "5.0.6",
"@lwc/engine-server": "5.0.6",
"@lwc/rollup-plugin": "5.0.6",
"@lwc/synthetic-shadow": "5.0.6",
"@lwc/compiler": "5.0.7",
"@lwc/engine-dom": "5.0.7",
"@lwc/engine-server": "5.0.7",
"@lwc/rollup-plugin": "5.0.7",
"@lwc/synthetic-shadow": "5.0.7",
"chokidar": "^3.5.3",
"istanbul-lib-coverage": "^3.2.2",
"istanbul-lib-report": "^3.0.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,7 @@ const GLOBAL_HTML_ATTRIBUTES = [
'spellcheck',
'tabIndex',
'title',
// Copy over all aria props supported on Element.prototype. Note that this will vary from browser to browser.
// See: https://wicg.github.io/aom/spec/aria-reflection.html
...Object.keys(Element.prototype).filter((prop) => ariaProperties.includes(prop)),
...ariaProperties,
].sort();

const message = (propName) =>
Expand Down
Loading

0 comments on commit 23ee7fd

Please sign in to comment.