Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
- Throw on repeat call to `attachInternals()`.
- Add warning for `checkValidity()` calls.
- Retype and rename `ariaMixinEnum` to `ariaMixinAttributes`.
- Loop through ariaAttrMap instead of internals instance.
  • Loading branch information
augustjk committed Mar 21, 2023
1 parent dcef2b4 commit 056a39c
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 10 deletions.
8 changes: 7 additions & 1 deletion packages/labs/ssr-dom-shim/src/index.ts
Expand Up @@ -6,7 +6,7 @@
import {ElementInternalsShim} from './lib/element-internals.js';

export {
ariaMixinEnum,
ariaMixinAttributes,
ElementInternals,
HYDRATE_INTERNALS_ATTR_PREFIX,
} from './lib/element-internals.js';
Expand Down Expand Up @@ -73,6 +73,12 @@ const ElementShim = class Element {
return shadowRoot;
}
attachInternals(): ElementInternals {
if (this.__internals !== null) {
throw new Error(
`Failed to execute 'attachInternals' on 'HTMLElement': ` +
`ElementInternals for the specified element was already attached.`
);
}
const internals = new ElementInternalsShim(this as unknown as HTMLElement);
this.__internals = internals;
return internals as ElementInternals;
Expand Down
15 changes: 14 additions & 1 deletion packages/labs/ssr-dom-shim/src/lib/element-internals.ts
Expand Up @@ -16,7 +16,14 @@ declare global {
}
}

export const ariaMixinEnum: Record<keyof ARIAMixin, string> = {
type ARIAAttributeMap = {
[K in keyof ARIAMixin]: string;
};

/**
* Map of ARIAMixin properties to attributes
*/
export const ariaMixinAttributes: ARIAAttributeMap = {
ariaAtomic: 'aria-atomic',
ariaAutoComplete: 'aria-autocomplete',
ariaBraileLabel: 'aria-brailelabel',
Expand Down Expand Up @@ -119,6 +126,12 @@ export const ElementInternalsShim = class ElementInternals
this.__host = _host;
}
checkValidity() {
// TODO(augustjk) Consider actually implementing logic.
// See https://github.com/lit/lit/issues/3740
console.warn(
'`ElementInternals.checkValidity()` was called on the server.' +
'This method always returns true.'
);
return true;
}
form = null;
Expand Down
14 changes: 6 additions & 8 deletions packages/labs/ssr/src/lib/lit-element-renderer.ts
Expand Up @@ -8,7 +8,7 @@ import {ElementRenderer} from './element-renderer.js';
import {LitElement, CSSResult, ReactiveElement} from 'lit';
import {_$LE} from 'lit-element/private-ssr-support.js';
import {
ariaMixinEnum,
ariaMixinAttributes,
HYDRATE_INTERNALS_ATTR_PREFIX,
} from '@lit-labs/ssr-dom-shim';
import {renderValue} from './render-value.js';
Expand Down Expand Up @@ -43,13 +43,11 @@ export class LitElementRenderer extends ElementRenderer {
this.element as object as {__internals: ElementInternals}
).__internals;
if (internals) {
for (const [key, value] of Object.entries(internals)) {
const ariaAttribute = ariaMixinEnum[key as keyof ARIAMixin];
if (
ariaAttribute &&
value &&
!this.element.hasAttribute(ariaAttribute)
) {
for (const [ariaProp, ariaAttribute] of Object.entries(
ariaMixinAttributes
)) {
const value = internals[ariaProp as keyof ARIAMixin];
if (value && !this.element.hasAttribute(ariaAttribute)) {
this.element.setAttribute(ariaAttribute, value);
this.element.setAttribute(
`${HYDRATE_INTERNALS_ATTR_PREFIX}${ariaAttribute}`,
Expand Down

0 comments on commit 056a39c

Please sign in to comment.