Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Lit] Fix hydration not having the same reactive values as server #6080

Merged
merged 8 commits into from
Feb 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/dry-sloths-flash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/lit': patch
---

Fixes Lit hydration not having the same reactive values as server (losing state upon hydration)
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { LitElement, html } from 'lit';

export default class NonDeferredCounter extends LitElement {
static get properties() {
return {
count: {
type: Number,
// All set properties are reflected to attributes so its hydration is
// not deferred.
reflect: true,
},
};
}

constructor() {
super();
this.count = 0;
}

increment() {
this.count++;
}

render() {
return html`
<div>
<p>Count: ${this.count}</p>

<button type="button" @click=${this.increment}>Increment</button>
</div>
`;
}
}

customElements.define('non-deferred-counter', NonDeferredCounter);
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
---
import MyCounter from '../components/Counter.js';
import NonDeferredCounter from '../components/NonDeferredCounter.js';

const someProps = {
count: 0,
count: 10,
};
---

Expand All @@ -15,6 +16,9 @@ const someProps = {
<h1>Hello, client:idle!</h1>
</MyCounter>

<NonDeferredCounter id="non-deferred" client:idle {...someProps}>
</NonDeferredCounter>

<MyCounter id="client-load" {...someProps} client:load>
<h1>Hello, client:load!</h1>
</MyCounter>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import MyCounter from '../components/Counter.js';

const someProps = {
count: 0,
count: 10,
};
---

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import MyCounter from '../components/Counter.js';

const someProps = {
count: 0,
count: 10,
};
---

Expand Down
31 changes: 22 additions & 9 deletions packages/astro/e2e/lit-component.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,25 @@ test.describe('Lit components', () => {
await expect(counter).toHaveCount(1);

const count = counter.locator('p');
await expect(count, 'initial count is 0').toHaveText('Count: 0');
await expect(count, 'initial count is 10').toHaveText('Count: 10');

const inc = counter.locator('button');
await inc.click();

await expect(count, 'count incremented by 1').toHaveText('Count: 1');
await expect(count, 'count incremented by 1').toHaveText('Count: 11');
});

t('non-deferred attribute serialization', async ({ page, astro }) => {
await page.goto(astro.resolveUrl('/'));

const counter = page.locator('#non-deferred');
const count = counter.locator('p');
await expect(count, 'initial count is 10').toHaveText('Count: 10');

const inc = counter.locator('button');
await inc.click();

await expect(count, 'count incremented by 1').toHaveText('Count: 11');
});

t('client:load', async ({ page, astro }) => {
Expand All @@ -47,12 +60,12 @@ test.describe('Lit components', () => {
await expect(counter, 'component is visible').toBeVisible();

const count = counter.locator('p');
await expect(count, 'initial count is 0').toHaveText('Count: 0');
await expect(count, 'initial count is 10').toHaveText('Count: 10');

const inc = counter.locator('button');
await inc.click();

await expect(count, 'count incremented by 1').toHaveText('Count: 1');
await expect(count, 'count incremented by 1').toHaveText('Count: 11');
});

t('client:visible', async ({ page, astro }) => {
Expand All @@ -64,12 +77,12 @@ test.describe('Lit components', () => {
await expect(counter, 'component is visible').toBeVisible();

const count = counter.locator('p');
await expect(count, 'initial count is 0').toHaveText('Count: 0');
await expect(count, 'initial count is 10').toHaveText('Count: 10');

const inc = counter.locator('button');
await inc.click();

await expect(count, 'count incremented by 1').toHaveText('Count: 1');
await expect(count, 'count incremented by 1').toHaveText('Count: 11');
});

t('client:media', async ({ page, astro }) => {
Expand All @@ -79,18 +92,18 @@ test.describe('Lit components', () => {
await expect(counter, 'component is visible').toBeVisible();

const count = counter.locator('p');
await expect(count, 'initial count is 0').toHaveText('Count: 0');
await expect(count, 'initial count is 10').toHaveText('Count: 10');

const inc = counter.locator('button');
await inc.click();

await expect(count, 'component not hydrated yet').toHaveText('Count: 0');
await expect(count, 'component not hydrated yet').toHaveText('Count: 10');

// Reset the viewport to hydrate the component (max-width: 50rem)
await page.setViewportSize({ width: 414, height: 1124 });

await inc.click();
await expect(count, 'count incremented by 1').toHaveText('Count: 1');
await expect(count, 'count incremented by 1').toHaveText('Count: 11');
});

t.skip('HMR', async ({ page, astro }) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { LitElement, html } from 'lit';
import { property, customElement } from 'lit/decorators.js';

@customElement('non-deferred-counter')
export class NonDeferredCounter extends LitElement {
// All set properties are reflected to attributes so its hydration is not
// hydration-deferred should always be set.
@property({ type: Number, reflect: true }) count = 0;

increment() {
this.count++;
}

render() {
return html`
<div>
<p>Count: ${this.count}</p>

<button type="button" @click=${this.increment}>Increment</button>
</div>
`;
}
}
23 changes: 15 additions & 8 deletions packages/astro/test/fixtures/lit-element/src/pages/index.astro
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
---
import {MyElement} from '../components/my-element.js';
import {NonDeferredCounter} from '../components/non-deferred-element.js';
---

<html>
<head>
<title>LitElements</title>
<title>LitElements</title>
</head>
<body>
<MyElement
foo="bar"
str-attr={'initialized'}
bool={false}
obj={{data: 1}}
reflectedStrProp={'initialized reflected'}>
</MyElement>
<MyElement
id="default"
foo="bar"
str-attr={'initialized'}
bool={false}
obj={{data: 1}}
reflectedStrProp={'initialized reflected'}>
</MyElement>
<NonDeferredCounter
id="non-deferred"
count={10}
foo="bar">
</NonDeferredCounter>
</body>
</html>
47 changes: 35 additions & 12 deletions packages/astro/test/lit-element.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,36 +30,59 @@ describe('LitElement test', function () {
const $ = cheerio.load(html);

// test 1: attributes rendered – non reactive properties
expect($('my-element').attr('foo')).to.equal('bar');
expect($('#default').attr('foo')).to.equal('bar');

// test 2: shadow rendered
expect($('my-element').html()).to.include(`<div>Testing...</div>`);
expect($('#default').html()).to.include(`<div>Testing...</div>`);

// test 3: string reactive property set
expect(stripExpressionMarkers($('my-element').html())).to.include(
expect(stripExpressionMarkers($('#default').html())).to.include(
`<div id="str">initialized</div>`
);

// test 4: boolean reactive property correctly set
// <my-element bool="false"> Lit will equate to true because it uses
// this.hasAttribute to determine its value
expect(stripExpressionMarkers($('my-element').html())).to.include(`<div id="bool">B</div>`);
expect(stripExpressionMarkers($('#default').html())).to.include(`<div id="bool">B</div>`);

// test 5: object reactive property set
// by default objects will be stringified to [object Object]
expect(stripExpressionMarkers($('my-element').html())).to.include(
expect(stripExpressionMarkers($('#default').html())).to.include(
`<div id="data">data: 1</div>`
);

// test 6: reactive properties are not rendered as attributes
expect($('my-element').attr('obj')).to.equal(undefined);
expect($('my-element').attr('bool')).to.equal(undefined);
expect($('my-element').attr('str')).to.equal(undefined);
expect($('#default').attr('obj')).to.equal(undefined);
expect($('#default').attr('bool')).to.equal(undefined);
expect($('#default').attr('str')).to.equal(undefined);

// test 7: reflected reactive props are rendered as attributes
expect($('my-element').attr('reflectedbool')).to.equal('');
expect($('my-element').attr('reflected-str')).to.equal('default reflected string');
expect($('my-element').attr('reflected-str-prop')).to.equal('initialized reflected');
expect($('#default').attr('reflectedbool')).to.equal('');
expect($('#default').attr('reflected-str')).to.equal('default reflected string');
expect($('#default').attr('reflected-str-prop')).to.equal('initialized reflected');
});

it('Sets defer-hydration on element only when necessary', async () => {
// @lit-labs/ssr/ requires Node 13.9 or higher
if (NODE_VERSION < 13.9) {
return;
}
const html = await fixture.readFile('/index.html');
const $ = cheerio.load(html);

// test 1: reflected reactive props are rendered as attributes
expect($('#non-deferred').attr('count')).to.equal('10');

// test 2: non-reactive props are set as attributes
expect($('#non-deferred').attr('foo')).to.equal('bar');

// test 3: components with only reflected reactive props set are not
// deferred because their state can be completely serialized via attributes
expect($('#non-deferred').attr('defer-hydration')).to.equal(undefined);

// test 4: components with non-reflected reactive props set are deferred because
// their state needs to be synced with the server on the client.
expect($('#default').attr('defer-hydration')).to.equal('');
});

it('Correctly passes child slots', async () => {
Expand All @@ -74,7 +97,7 @@ describe('LitElement test', function () {
const $slottedMyElement = $('#slotted');
const $slottedSlottedMyElement = $('#slotted-slotted');

expect($('my-element').length).to.equal(3);
expect($('#default').length).to.equal(3);

// Root my-element
expect($rootMyElement.children('.default').length).to.equal(2);
Expand Down
1 change: 1 addition & 0 deletions packages/integrations/lit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
".": "./dist/index.js",
"./server.js": "./server.js",
"./client-shim.js": "./client-shim.js",
"./dist/client.js": "./dist/client.js",
"./hydration-support.js": "./hydration-support.js",
"./package.json": "./package.json"
},
Expand Down
14 changes: 11 additions & 3 deletions packages/integrations/lit/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,18 @@ function* render(Component, attrs, slots) {

// LitElementRenderer creates a new element instance, so copy over.
const Ctr = getCustomElementConstructor(tagName);
let shouldDeferHydration = false;

if (attrs) {
for (let [name, value] of Object.entries(attrs)) {
// check if this is a reactive property
if (name in Ctr.prototype) {
const isReactiveProperty = name in Ctr.prototype;
const isReflectedReactiveProperty = Ctr.elementProperties.get(name)?.reflect;

// Only defer hydration if we are setting a reactive property that cannot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love these comments

// be reflected / serialized as a property.
shouldDeferHydration ||= isReactiveProperty && !isReflectedReactiveProperty;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you might be the first to add logical or assignments to our codebase.


if (isReactiveProperty) {
instance.setProperty(name, value);
} else {
instance.setAttribute(name, value);
Expand All @@ -49,7 +57,7 @@ function* render(Component, attrs, slots) {

instance.connectedCallback();

yield `<${tagName}`;
yield `<${tagName}${shouldDeferHydration ? ' defer-hydration' : ''}`;
yield* instance.renderAttributes();
yield `>`;
const shadowContents = instance.renderShadow({});
Expand Down
25 changes: 25 additions & 0 deletions packages/integrations/lit/src/client.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
export default (element: HTMLElement) =>
async (
Component: any,
props: Record<string, any>,
) => {
// Get the LitElement element instance (may or may not be upgraded).
const component = element.children[0] as HTMLElement;

// If there is no deferral of hydration, then all reactive properties are
// already serialzied as reflected attributes, or no reactive props were set
if (!component || !component.hasAttribute('defer-hydration')) {
return;
}

// Set properties on the LitElement instance for resuming hydration.
for (let [name, value] of Object.entries(props)) {
// Check if reactive property or class property.
if (name in Component.prototype) {
(component as any)[name] = value;
}
}

// Tell LitElement to resume hydration.
component.removeAttribute('defer-hydration');
};
2 changes: 2 additions & 0 deletions packages/integrations/lit/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ function getViteConfiguration() {
return {
optimizeDeps: {
include: [
'@astrojs/lit/dist/client.js',
'@astrojs/lit/client-shim.js',
'@astrojs/lit/hydration-support.js',
'@webcomponents/template-shadowroot/template-shadowroot.js',
Expand Down Expand Up @@ -34,6 +35,7 @@ export default function (): AstroIntegration {
addRenderer({
name: '@astrojs/lit',
serverEntrypoint: '@astrojs/lit/server.js',
clientEntrypoint: '@astrojs/lit/dist/client.js',
});
// Update the vite configuration.
updateConfig({
Expand Down