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

feat(core): slot controller ssr hint attributes #2893

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

bennypowers
Copy link
Member

@bennypowers bennypowers commented Jan 30, 2025

Follow up to #2891

Lit-SSR forbids accessing DOM children when rendering the shadow root of an element server side, but we are permitted to access the attributes, so this PR adds ssr-hint attributes for slotted content, so that elements which use SlotController to render the shadowroot differently depending on which slots have content will work. Pages that load the SSR shim that don't solve this problem will be broken because of hydration mismatch errors. I've yet to find a workaround which can fix an element broken in that way after the fact.

What I did

  1. Add a server-side only slotcontroller
  2. add ssr-hint-has-slotted and ssr-hint-has-default-slot attributes

Users doing SSR on the page would be required to ship source HTML with ssr hint attributes, for example (using pf-card)

<pf-card ssr-hint-has-default-slotted ssr-hint-has-slotted="header,footer">
  <h2 slot="header">Header slot content</h2>
  <p>Default slot content</p>
  <pf-button slot="footer">Footer slot content</pf-button>
</pf-card>

Failure to add the ssr-hint-* attributes, or adding incorrect attributes (e.g. saying there's content for a slot when there isn't or vice versa) will result in a broken element.

Automations could be written to apply those attributes to elements server-side prior to SSR'ing the page, e.g. in an eleventy transform.

Testing Instructions

  1. feat: ssr-friendly slot-controller #2505 has some code which adds tests for this, which ought to be backported into this branch. For now if you're interested, run the build on this branch, then copy the files into the RHDS repo and test there. e.g.
npm run build
cp core/pfe-core/controllers/slot-controller*.{js,d.ts} ../red-hat-design-system/node_modules/@patternfly/pfe-core/controllers

See also RedHat-UX/red-hat-design-system#2131, specifically elements/rh-card/demo/darkest.html

Notes to Reviewers

  1. Please consider the design here, can you suggest an alternative approach? what do you think are the major blockers to this solution, if any?

Copy link

changeset-bot bot commented Jan 30, 2025

🦋 Changeset detected

Latest commit: 9f5ecaa

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@patternfly/pfe-core Patch
@patternfly/elements Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Jan 30, 2025

✅ Commitlint tests passed!

More Info
{
  "valid": true,
  "errors": [],
  "warnings": [],
  "input": "feat(core): slot controller ssr hint attributes"
}

Copy link

netlify bot commented Jan 30, 2025

Deploy Preview for patternfly-elements ready!

Name Link
🔨 Latest commit 0495048
😎 Deploy Preview https://deploy-preview-2893--patternfly-elements.netlify.app/

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions github-actions bot added the AT passed Automated testing has passed label Jan 30, 2025
Copy link
Contributor

SSR Test Run for e2f89a0: Report

Copy link

netlify bot commented Jan 30, 2025

Deploy Preview for patternfly-elements ready!

Name Link
🔨 Latest commit 9f5ecaa
🔍 Latest deploy log https://app.netlify.com/sites/patternfly-elements/deploys/679bab01d015cc00081fe7a6
😎 Deploy Preview https://deploy-preview-2893--patternfly-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

SSR Test Run for 0495048: Report

@heyMP
Copy link
Contributor

heyMP commented Jan 30, 2025

It's such a bummer that Lit-SSR doesn't support reading children. I get it but it's unfortunate. I think it would be a great feature if you could disable "streaming mode" on Lit-SSR and you'd have access to the whole DOM content.

I think this fix is really creative and I think it's a good solution. I do worry though about making it mandatory for implementors who are trying to pre-render. I can see more instances of these ssr-hint-* attributes in the future to get around the Lit-SSR issue.

I also wonder if we could reduce the need for slot-controller by utilizing the CSS :has() selector. For instance, if you're relying on hiding and showing certain pieces of the shadowroot based on what slots are in the lightdom, I think that could be a better use case for CSS :has() over slot-controller. It would remove the requirement for ssr-hint-* attributes for that specific component. Just thinking out loud. 😀

@zhawkins
Copy link
Contributor

Very creative for sure. That said, I share Potters concerns about tackling it with attributes.

Can you share an example or two of

elements which use SlotController to render the shadowroot differently depending on which slots have content

Curious to know what that looks like and if a CSS solution like Potter touched on might be viable. 🤔

@bennypowers
Copy link
Member Author

Light DOM CSS solves a different problem than this PR.

Light DOM CSS is only for styling deeply nested children, since the alternative (slapping style attributes on children via the DOM API) is truly unpalatable.

This PR is primarily about ponyfilling :has-slotted for shadow stylesheets via shadow classes, but not exclusively.

#slots = new SlotController(this, 'belly');

render() {
  return html`
    <div id="stomach" class="${classMap({ empty: this.#slots.isEmpty('belly') })}">
      <belly-button>
      <slot name="belly"></slot>
    </div>
    <!-- head, shoulders, knees, toes -->
  `;
}

This case shows how using slotcontroller we could hide or otherwise style the container for the belly slot when there's no slotted belly content. How would you handle this case with light dom css? You couldn't without exposing a shadow part.

So why not just expose a shadow part then? That would be applying a solution in the element's design to solve a technical problem in the element's implementation. that's a severe inversion of priorities.

The solution we're looking for is something like:

#stomach:has(:has-slotted(*)) {
  /* ... */
}

but that's not quite ready yet, so we need a stopgap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AT passed Automated testing has passed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants