Skip to content

Commit

Permalink
fix(context): context nested grandchild (#1985)
Browse files Browse the repository at this point in the history
* docs(surface): nested grandchild demo

* fix(context): workarounds for SSR scenarios

although calling the provider's update method on every element update is
overkill, this prevents bugs in certain ssr scenarios

* fix(cta): fix some hydration errors

* docs: fix config
  • Loading branch information
bennypowers authored Oct 14, 2024
1 parent a4973b9 commit f63c46d
Show file tree
Hide file tree
Showing 13 changed files with 119 additions and 66 deletions.
4 changes: 4 additions & 0 deletions .changeset/early-eels-fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
"@rhds/elements": patch
---
**Color Context**: prevent errors in certain SSR scenarios
4 changes: 4 additions & 0 deletions .changeset/true-doors-leave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
"@rhds/elements": patch
---
`<rh-cta>`: fix some errors when hydrating in SSR scenarios
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ docs/assets/javascript/environment.js
elements.js
elements/*/*.js
elements/*/test/*.js
uxdot/*.js
react
lib/**/*.js
!elements/**/demo/*.css
Expand Down
5 changes: 5 additions & 0 deletions docs/patterns/card/patterns/themes.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,8 @@ <h2 slot="header">Card title</h2>
elit. Donec id elit non mi porta gravida at eget metus.</p>
<rh-cta slot="footer" href="#">Call to action</rh-cta>
</rh-card>

<script type="module">
import '@rhds/elements/rh-card/rh-card.js';
import '@rhds/elements/rh-cta/rh-cta.js';
</script>
15 changes: 15 additions & 0 deletions docs/patterns/tabs/patterns/link-to-tab.html
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,18 @@ <h3 id="simulate-nav-heading">Navigate to tab</h3>
activateTabByHash();
</script>

<style>
a {
color: var(--rh-color-interactive-primary-default);
&:hover { color: var(--rh-color-interactive-primary-hover); }
&:focus-within { color: var(--rh-color-interactive-primary-focus); }
&:active { color: var(--rh-color-interactive-primary-active); }
&:visited {
color: var(--rh-color-interactive-primary-visited);
&:hover { color: var(--rh-color-interactive-primary-visited-hover); }
&:focus-within { color: var(--rh-color-interactive-primary-visited-focus); }
&:active { color: var(--rh-color-interactive-primary-visited-active); }
}
}
</style>

2 changes: 1 addition & 1 deletion elements/rh-cta/demo/variants.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

<div id="brick">
<rh-cta variant="brick" href="#brick">Brick</rh-cta>
<rh-cta variant="brick" icon="user" href="#brick-icon">Brick Icon</rh-cta>
<rh-cta variant="brick" icon="users" href="#brick-icon">Brick Icon</rh-cta>
</div>
</section>

Expand Down
42 changes: 24 additions & 18 deletions elements/rh-cta/rh-cta.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { LitElement, html } from 'lit';
import { LitElement, html, isServer, noChange } from 'lit';
import { customElement } from 'lit/decorators/custom-element.js';
import { property } from 'lit/decorators/property.js';
import { classMap } from 'lit/directives/class-map.js';
Expand All @@ -13,6 +13,7 @@ import { colorContextConsumer, type ColorTheme } from '../../lib/context/color/c
import type { IconNameFor, IconSetName } from '@rhds/icons';

import style from './rh-cta.css';
import { state } from 'lit/decorators/state.js';

function isSupportedContent(el: Element | null): el is HTMLAnchorElement | HTMLButtonElement {
return el instanceof HTMLAnchorElement || el instanceof HTMLButtonElement;
Expand Down Expand Up @@ -72,8 +73,6 @@ function isSupportedContent(el: Element | null): el is HTMLAnchorElement | HTMLB
*/
@customElement('rh-cta')
export class RhCta extends LitElement {
static readonly version = '{{version}}';

static readonly styles = [style];

/**
Expand Down Expand Up @@ -120,7 +119,7 @@ export class RhCta extends LitElement {
/**
* Sets color theme based on parent context
*/
@colorContextConsumer() private on?: ColorTheme;
@colorContextConsumer() @state() private on?: ColorTheme;

protected override async getUpdateComplete(): Promise<boolean> {
if (this.icon || !this.variant) {
Expand All @@ -144,28 +143,35 @@ export class RhCta extends LitElement {
const isDefault = !variant;
const svg = isDefault;
const iconOrSvg = isDefault || !!icon;
const follower = !iconOrSvg ? '' : variant !== 'brick' && icon ? html`<!--
--><rh-icon icon=${icon} set=${iconSet ?? 'ui'}></rh-icon>` : variant ? '' : html`<!--
--><rh-icon set="ui" icon="arrow-right"></rh-icon>`;
const follower =
(variant !== 'brick' && icon) ?
html`<rh-icon .icon=${icon} .set=${iconSet ?? 'ui'}></rh-icon>`
: (variant === undefined) ?
html`<rh-icon set="ui" icon="arrow-right"></rh-icon>`
: html``;
const iconContent =
(variant === 'brick' && icon) ?
html`<rh-icon .icon=${icon} set="${iconSet ?? 'ui'}"></rh-icon>`
: html``;
const linkContent =
!href ? html`<slot></slot>${follower}`
: html`<a href=${href}
download="${ifDefined(download)}"
rel="${ifDefined(rel)}"
referrerpolicy="${ifDefined(referrerpolicy)}"
target="${ifDefined(target)}"><slot></slot>${follower}</a>`;
return html`
<span id="container"
part="container"
class=${classMap({ rtl, icon: !!icon, svg, on: true, [on]: true })}
@slotchange=${this.firstUpdated}>${variant === 'brick' && icon ? html`
<rh-icon icon=${icon}
set="${iconSet ?? 'ui'}"></rh-icon>` : ''}${href ? html`
<a href=${href}
download="${ifDefined(download)}"
rel="${ifDefined(rel)}"
referrerpolicy="${ifDefined(referrerpolicy)}"
target="${ifDefined(target)}"><slot></slot>${follower}</a>`
: html`<slot></slot>${follower}`}
</span>
`;
@slotchange=${this.firstUpdated}>${iconContent}${linkContent}</span>`;
}

override firstUpdated() {
const { href, variant } = this;
if (!isServer) {
this.removeAttribute('defer-hydration');
}
const cta =
this.shadowRoot?.querySelector('a')
?? this.shadowRoot?.querySelector('slot')?.assignedElements().find(isSupportedContent)
Expand Down
13 changes: 13 additions & 0 deletions elements/rh-surface/demo/nested-combination-elements.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<rh-surface color-palette="darkest">
<rh-card>
<p>The card has no color-palette. It's nested CTA should therefore inherit
context from the grandparent, rh-surface.</p>
<rh-cta href="#">Should be on dark</rh-cta>
</rh-card>
</rh-surface>

<script type="module">
import '@rhds/elements/rh-surface/rh-surface.js';
import '@rhds/elements/rh-card/rh-card.js';
import '@rhds/elements/rh-cta/rh-cta.js';
</script>
63 changes: 29 additions & 34 deletions eleventy.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ import RHDSMarkdownItPlugin from '#11ty-plugins/markdown-it.js';
import ImportMapPlugin from '#11ty-plugins/importMap.js';
import LitPlugin from '#11ty-plugins/lit-ssr/lit.js';

import { promisify } from 'node:util';
import * as ChildProcess from 'node:child_process';
import { $ } from 'execa';

export interface GlobalData {
runMode: 'build' | 'watch' | 'serve';
Expand All @@ -43,8 +42,6 @@ export class Renderer<T> {
render?(data: T & GlobalData): string | Promise<string>;
}

const exec = promisify(ChildProcess.exec);

const isWatch =
process.argv.includes('--serve') || process.argv.includes('--watch');

Expand All @@ -57,15 +54,9 @@ export default async function(eleventyConfig: UserConfig) {
eleventyConfig.addGlobalData('runMode', runMode);
});

eleventyConfig.on('eleventy.before', async function() {
const { stdout, stderr } = await exec('npx tspc -b');
if (stderr) {
// eslint-disable-next-line no-console
console.error(stderr);
} else {
// eslint-disable-next-line no-console
console.log(stdout);
}
let watch;
eleventyConfig.on('eleventy.before', function({ runMode }) {
watch ||= runMode === 'watch' && $({ stdout: ['pipe'], stderr: ['pipe'] })`npx tspc -b --watch`;
});

eleventyConfig.watchIgnores?.add('docs/assets/redhat/');
Expand All @@ -74,6 +65,7 @@ export default async function(eleventyConfig: UserConfig) {
eleventyConfig.watchIgnores?.add('**/*.js.map');
eleventyConfig.watchIgnores?.add('elements/*/test/');
eleventyConfig.watchIgnores?.add('lib/elements/*/test/');
eleventyConfig.watchIgnores?.add('**/*.tsbuildinfo');
eleventyConfig.addPassthroughCopy('docs/patterns/**/*.{svg,jpg,jpeg,png}');
eleventyConfig.addPassthroughCopy('elements/*/demo/**/*.{svg,jpg,jpeg,png}');
eleventyConfig.addPassthroughCopy('docs/CNAME');
Expand Down Expand Up @@ -121,10 +113,13 @@ export default async function(eleventyConfig: UserConfig) {
eleventyConfig.addPassthroughCopy({
'node_modules/@lit/reactive-element': '/assets/packages/@lit/reactive-element',
});
const isNotTsbuild = (p: string) => !p.includes('.');
eleventyConfig.addPassthroughCopy({ 'elements': `/assets/packages/@rhds/elements/elements/` }, { filter: isNotTsbuild });
eleventyConfig.addPassthroughCopy({ 'lib': `/assets/packages/@rhds/elements/lib/` }, { filter: isNotTsbuild });
eleventyConfig.addPassthroughCopy({ 'uxdot': `/assets/packages/@uxdot/elements/` }, { filter: isNotTsbuild });
eleventyConfig.addPassthroughCopy({
'elements': `/assets/packages/@rhds/elements/elements/`,
'lib': `/assets/packages/@rhds/elements/lib/`,
'uxdot': `/assets/packages/@uxdot/elements/`,
}, {
filter: (p: string) => !p.startsWith('tsconfig'),
});
eleventyConfig.addPlugin(ImportMapPlugin, {
nodemodulesPublicPath: '/assets/packages',
manualImportMap: {
Expand All @@ -145,7 +140,7 @@ export default async function(eleventyConfig: UserConfig) {
'@patternfly/elements/': '/assets/packages/@patternfly/elements/',
'@patternfly/icons/': '/assets/packages/@patternfly/icons/',
'@patternfly/pfe-core/': '/assets/packages/@patternfly/pfe-core/',
'@uxdot/elements/': '/assets/javascript/elements/uxdot/',
'@uxdot/elements/': '/assets/packages/@uxdot/elements/',
'playground-elements': 'https://cdn.jsdelivr.net/npm/[email protected]/+esm',
},
},
Expand Down Expand Up @@ -225,22 +220,22 @@ export default async function(eleventyConfig: UserConfig) {

eleventyConfig.addPlugin(LitPlugin, {
componentModules: [
'docs/assets/javascript/elements/uxdot/uxdot-best-practice.js',
'docs/assets/javascript/elements/uxdot/uxdot-copy-button.js',
'docs/assets/javascript/elements/uxdot/uxdot-copy-permalink.js',
'docs/assets/javascript/elements/uxdot/uxdot-example.js',
'docs/assets/javascript/elements/uxdot/uxdot-feedback.js',
'docs/assets/javascript/elements/uxdot/uxdot-header.js',
'docs/assets/javascript/elements/uxdot/uxdot-hero.js',
'docs/assets/javascript/elements/uxdot/uxdot-installation-tabs.js',
'docs/assets/javascript/elements/uxdot/uxdot-masthead.js',
'docs/assets/javascript/elements/uxdot/uxdot-pattern.js',
'docs/assets/javascript/elements/uxdot/uxdot-repo-status-checklist.js',
'docs/assets/javascript/elements/uxdot/uxdot-repo-status-list.js',
'docs/assets/javascript/elements/uxdot/uxdot-search.js',
'docs/assets/javascript/elements/uxdot/uxdot-sidenav.js',
'docs/assets/javascript/elements/uxdot/uxdot-spacer-tokens-table.js',
'docs/assets/javascript/elements/uxdot/uxdot-toc.js',
'uxdot/uxdot-best-practice.js',
'uxdot/uxdot-copy-button.js',
'uxdot/uxdot-copy-permalink.js',
'uxdot/uxdot-example.js',
'uxdot/uxdot-feedback.js',
'uxdot/uxdot-header.js',
'uxdot/uxdot-hero.js',
'uxdot/uxdot-installation-tabs.js',
'uxdot/uxdot-masthead.js',
'uxdot/uxdot-pattern.js',
'uxdot/uxdot-repo-status-checklist.js',
'uxdot/uxdot-repo-status-list.js',
'uxdot/uxdot-search.js',
'uxdot/uxdot-sidenav.js',
'uxdot/uxdot-spacer-tokens-table.js',
'uxdot/uxdot-toc.js',
'elements/rh-button/rh-button.js',
'elements/rh-code-block/rh-code-block.js',
'elements/rh-footer/rh-footer-universal.js',
Expand Down
10 changes: 9 additions & 1 deletion lib/context/color/consumer.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { ReactiveController, ReactiveElement } from 'lit';
import { isServer, type ReactiveController, type ReactiveElement } from 'lit';

import {
contextEvents,
Expand Down Expand Up @@ -69,6 +69,14 @@ export class ColorContextConsumer<
this.#override = null;
}

hostUpdated() {
if (!isServer && !this.host.hasUpdated) {
// This is definitely overkill, but it's the only
// way we've found so far to work around lit-ssr hydration woes
this.hostConnected();
}
}

/** When a consumer disconnects, it's removed from the list of consumers. */
hostDisconnected() {
this.#dispose?.();
Expand Down
20 changes: 13 additions & 7 deletions lib/context/color/provider.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { ReactiveController, ReactiveElement } from 'lit';
import { isServer, type ReactiveController, type ReactiveElement } from 'lit';
import type { ContextCallback, ContextRequestEvent, UnknownContext } from '../event.js';

import {
Expand Down Expand Up @@ -69,7 +69,6 @@ export class ColorContextProvider<
// eslint-disable-next-line no-unused-private-class-members
#style: CSSStyleDeclaration;

// eslint-disable-next-line no-unused-private-class-members
#initialized = false;

#logger: Logger;
Expand Down Expand Up @@ -116,14 +115,23 @@ export class ColorContextProvider<
}
await this.host.updateComplete;
this.update();
return true;
}

hostUpdated() {
this.#initialized ||= (this.update(), true);
async hostUpdated() {
if (!this.#initialized) {
this.hostConnected();
}
this.#initialized ||= await this.hostConnected();
if (this.#local && this.value !== this.#consumer.value) {
this.#consumer.update(this.#local);
this.update();
}
if (!isServer) {
// This is definitely overkill, but it's the only
// way we've found so far to work around lit-ssr hydration woes
this.update();
}
}

/**
Expand Down Expand Up @@ -171,10 +179,8 @@ export class ColorContextProvider<
* @param [force] override theme
*/
public override async update(force?: ColorTheme) {
const { value } = this;

for (const cb of this.#callbacks) {
cb(force ?? value);
cb(force ?? this.value);
}
}
}
Expand Down
3 changes: 0 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,6 @@
"dependencies": [
"analyze",
"compile"
],
"output": [
"_site"
]
},
"e2e": {
Expand Down
3 changes: 1 addition & 2 deletions uxdot/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
"../docs/11ty-types.d.ts"
],
"compilerOptions": {
"composite": true,
"outDir": "../docs/assets/javascript/elements"
"composite": true
}
}

0 comments on commit f63c46d

Please sign in to comment.