Skip to content

Commit

Permalink
Change the type of renderRoot to include Element (#4254)
Browse files Browse the repository at this point in the history
* Use a Mutable<T, K> type to more accurate casts

* Change the type of renderRoot to include Element

* Change createRenderRoot too

* Update check-size script

* Update check-size.js

---------

Co-authored-by: Andrew Jakubowicz <ajakubowicz@google.com>
Co-authored-by: Peter Burns <rictic@google.com>
  • Loading branch information
3 people authored Oct 10, 2023
1 parent f14fc2f commit 1040f75
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 15 deletions.
7 changes: 7 additions & 0 deletions .changeset/fresh-goats-suffer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@lit/reactive-element': major
'lit-element': major
'lit': major
---

Change the type of `ReactiveElement.renderRoot` and return type of `ReactiveElement.createRenderRoot()` to be `HTMLElement | DocumentFragment` to match each other and lit-html's `render()` method.
32 changes: 18 additions & 14 deletions packages/reactive-element/src/reactive-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,17 @@ export type {
ReactiveControllerHost,
} from './reactive-controller.js';

/**
* Removes the `readonly` modifier from properties in the union K.
*
* This is a safer way to cast a value to a type with a mutable version of a
* readonly field, than casting to an interface with the field re-declared
* because it preserves the type of all the fields and warns on typos.
*/
type Mutable<T, K extends keyof T> = Omit<T, K> & {
-readonly [P in keyof Pick<T, K>]: P extends K ? T[P] : never;
};

// TODO (justinfagnani): Add `hasOwn` here when we ship ES2022
const {
is,
Expand Down Expand Up @@ -660,11 +671,9 @@ export abstract class ReactiveElement
name: PropertyKey,
options: PropertyDeclaration = defaultPropertyDeclaration
) {
// if this is a state property, force the attribute to false.
// If this is a state property, force the attribute to false.
if (options.state) {
// Cast as any since this is readonly.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(options as any).attribute = false;
(options as Mutable<PropertyDeclaration, 'attribute'>).attribute = false;
}
this.__prepare();
this.elementProperties.set(name, options);
Expand Down Expand Up @@ -925,7 +934,7 @@ export abstract class ReactiveElement
* to an open shadowRoot.
* @category rendering
*/
readonly renderRoot!: HTMLElement | ShadowRoot;
readonly renderRoot!: HTMLElement | DocumentFragment;

/**
* Returns the property name for the given attribute `name`.
Expand Down Expand Up @@ -1077,7 +1086,7 @@ export abstract class ReactiveElement
* @return Returns a node into which to render.
* @category rendering
*/
protected createRenderRoot(): Element | ShadowRoot {
protected createRenderRoot(): HTMLElement | DocumentFragment {
const renderRoot =
this.shadowRoot ??
this.attachShadow(
Expand All @@ -1096,14 +1105,9 @@ export abstract class ReactiveElement
* @category lifecycle
*/
connectedCallback() {
// create renderRoot before first update.
if (this.renderRoot === undefined) {
(
this as {
renderRoot: Element | DocumentFragment;
}
).renderRoot = this.createRenderRoot();
}
// Create renderRoot before first update.
(this as Mutable<typeof this, 'renderRoot'>).renderRoot ??=
this.createRenderRoot();
this.enableUpdating(true);
this.__controllers?.forEach((c) => c.hostConnected?.());
}
Expand Down
2 changes: 1 addition & 1 deletion scripts/check-size.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import * as fs from 'fs';
// it's likely that we'll ask you to investigate ways to reduce the size.
//
// In either case, update the size here and push a new commit to your PR.
const expectedLitCoreSize = 15467;
const expectedLitCoreSize = 15441;
const expectedLitHtmlSize = 7256;

const litCoreSrc = fs.readFileSync('packages/lit/lit-core.min.js', 'utf8');
Expand Down

0 comments on commit 1040f75

Please sign in to comment.