fix: use @lit/context#2131
Conversation
|
✅ Deploy Preview for red-hat-design-system ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Size Change: -4.53 kB (-2.19%) Total Size: 202 kB
ℹ️ View Unchanged
|
just riffing
|
at the moment, it looks like the callback for context consumers isn't being called. either i missed something simple or maybe there's an order of operations issue |
|
Ran some debug today. I believe we have a race condition in the order of imports. See the minimal repro below which successfully computes the context for a cta inside a card. the min repro has two major changes from prod:
Working minimal Reproduction caseimport type { LitElement, ReactiveController } from 'lit';
import type { RenderInfo, RenderResult } from '@lit-labs/ssr';
import type { RHDSSSRController } from '@rhds/elements/lib/ssr-controller.ts';
import '@patternfly/pfe-core/ssr-shims.js';
import '@lit-labs/ssr-dom-shim';
import { LitElementRenderer } from '@lit-labs/ssr/lib/lit-element-renderer.js';
import { writeFile } from 'node:fs/promises';
import { register } from 'node:module';
import { register as registerTS } from 'tsx/esm/api';
import { html } from 'lit';
import { render } from '@lit-labs/ssr';
import { collectResult } from '@lit-labs/ssr/lib/render-result.js';
const tsconfig = './tsconfig.settings.json';
register('./docs/_plugins/lit-ssr/lit-css-node.ts', import.meta.url);
registerTS({ tsconfig });
await import ('./elements/rh-card/rh-card.ts');
await import ('./elements/rh-cta/rh-cta.ts');
class RHDSSSRableRenderer extends LitElementRenderer {
static isRHDSSSRController(ctrl: ReactiveController): ctrl is RHDSSSRController {
return !!(ctrl as RHDSSSRController).isRHDSSSRController;
}
getControllers() {
const element = (this.element as LitElement & { _$EO: Set<ReactiveController> });
return Array.from(element._$EO ?? new Set())
.filter(RHDSSSRableRenderer.isRHDSSSRController);
}
async setupController(controller: RHDSSSRController, renderInfo: RenderInfo) {
if (controller.ssrSetup) {
await controller.ssrSetup(renderInfo);
}
return '';
}
override* renderShadow(renderInfo: RenderInfo): RenderResult {
for (const controller of this.getControllers()) {
yield this.setupController(controller, renderInfo);
}
yield* super.renderShadow(renderInfo);
}
}
class UnsafeHTMLStringsArray extends Array {
public raw: readonly string[];
constructor(string: string) {
super();
this.push(string);
this.raw = [string];
}
}
// Lit SSR includes comment markers to track the outer template from
// the template we've generated here, but it's not possible for this
// outer template to be hydrated, so they serve no purpose.
function trimOuterMarkers(renderedContent: string) {
return renderedContent
.replace(/^((<!--[^<>]*-->)|(<\?>)|\s)+/, '')
.replace(/((<!--[^<>]*-->)|(<\?>)|\s)+$/, '');
}
const tpl = html(new UnsafeHTMLStringsArray(/* html */`
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>context consumer noop repro</title>
<script type="importmap">
{
"imports": {
"@lit-labs/ssr-client/lit-element-hydrate-support.js": "/assets/packages/@lit-labs/ssr-client/lit-element-hydrate-support.js",
"@lit/context": "/assets/packages/@lit/context/index.js",
"@lit/reactive-element": "/assets/packages/@lit/reactive-element/reactive-element.js",
"@lit/reactive-element/decorators/": "/assets/packages/@lit/reactive-element/decorators/",
"@patternfly/elements": "/assets/packages/@patternfly/elements/pfe.min.js",
"@patternfly/elements/": "/assets/packages/@patternfly/elements/",
"@patternfly/icons/": "/assets/packages/@patternfly/icons/",
"@patternfly/pfe-core": "https://ga.jspm.io/npm:@patternfly/pfe-core@4.0.4/core.js",
"@patternfly/pfe-core/": "/assets/packages/@patternfly/pfe-core/",
"@rhds/elements/": "/assets/packages/@rhds/elements/elements/",
"@rhds/elements/lib/": "/assets/packages/@rhds/elements/lib/",
"@rhds/icons/": "/assets/packages/@rhds/icons/",
"@rhds/icons/icons.js": "/assets/packages/@rhds/icons/icons.js",
"@rhds/tokens/": "/assets/packages/@rhds/tokens/js/",
"@rhds/tokens/css/": "/assets/packages/@rhds/tokens/css/",
"lit": "/assets/packages/lit/index.js",
"lit-element": "/assets/packages/lit-element/index.js",
"lit-element/lit-element.js": "/assets/packages/lit-element/lit-element.js",
"lit-html": "/assets/packages/lit-html/lit-html.js",
"lit-html/": "/assets/packages/lit-html/",
"lit/": "/assets/packages/lit/",
"lit/decorators/custom-element.js": "/assets/packages/lit/decorators/custom-element.js",
"lit/decorators/property.js": "/assets/packages/lit/decorators/property.js",
"lit/directives/class-map.js": "/assets/packages/lit/directives/class-map.js",
"lit/directives/if-defined.js": "/assets/packages/lit/directives/if-defined.js",
"lit/directives/repeat.js": "/assets/packages/lit/directives/repeat.js",
"lit/static-html.js": "/assets/packages/lit/static-html.js",
"tslib": "/assets/packages/tslib/tslib.es6.mjs"
}
}
</script>
<script type="module">
import '/assets/javascript/dsd-polyfill.js';
import '@lit-labs/ssr-client/lit-element-hydrate-support.js';
import 'element-internals-polyfill';
import '@rhds/elements/rh-card/rh-card.js';
import '@rhds/elements/rh-cta/rh-cta.js';
</script>
</head>
<body>
<main>
<rh-card color-palette="darkest">
<rh-cta slot="footer" href="#"></rh-cta>
</rh-card>
</main>
</body>
</html>
`));
const result = render(tpl, { elementRenderers: [RHDSSSRableRenderer] });
const rendered = await collectResult(result);
await writeFile(
new URL('./_site/repro.html', import.meta.url),
trimOuterMarkers(rendered),
'utf8',
); |
|
as of 9f048b1, context basically works (barring some additional qe, the inevitable small bugs etc) However SSR is still fraught because the state of a SlotController can't easily be reset by client, leading to mismatches |
See #2131 which fixes this fundamentally
see #2131 for a comprehensive fix
* docs: new homepage * docs: improve homepage styles * docs: homepage header typography * style: whitespace * chore: update dependencies * chore: remove patches * chore: ease up on dep upgrades * chore: node version * chore: chill on updates moreso * chore: tokens version * chore: update tokens deps * style: whitespace * docs: homepage layout fixes * chore: update deps * docs: homepage layout and fonts * chore: deps * docs: remove global ts type from our custom context event See #2131 which fixes this fundamentally * fix: ts types see #2131 for a comprehensive fix * docs: homepage styles
|
Closing this in favor of smaller, focused PRs. The landscape has changed significantly since this was opened:
The remaining relevant work is:
Opening separate PRs for each. |
`updateAccessibility()` and `MutationObserver` require DOM APIs unavailable during server-side rendering. Guard them behind `isServer`. Closes part of #2131. Assisted-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fix(accordion): guard SSR-unsafe DOM operations in `connectedCallback` `updateAccessibility()` and `MutationObserver` require DOM APIs unavailable during server-side rendering. Guard them behind `isServer`. Closes part of #2131. Assisted-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This PR is based on and requires patternfly/patternfly-elements#2891
What I did
Testing Instructions
Notes to Reviewers
pay special attention to the heading level elements, this may require a wholesale refactor to get working under the new context regime.