tajo / ladle

🥄 Develop, test and document your React story components faster.
https://www.ladle.dev
MIT License
2.59k stars 87 forks source link

Styles inserted via CSSStyleSheet API don't survive cloning in iframe mode #207

Closed bensmithett closed 1 year ago

bensmithett commented 2 years ago

Describe the bug

When cloning style nodes in iframe mode (#196) styles inserted using the CSSStyleSheet API don't make it across.

I'm using Fela which uses CSSStyleSheet.insertRule.

StackOverflow answer that suggests looping through to copy each rule: https://stackoverflow.com/a/56156757

If I get some time before you get to this I'll poke around to see if there's a better way. Maybe something like grabbing document.styleSheets and using adoptedStyleSheets in the iframe might work?

Reproduction

https://stackblitz.com/edit/ladle-pxajhf?file=src%2Fbasic.stories.jsx

The text should be blue. When you set the story width it's red.

Environment

I don't think it's environment specific but I'm testing on

tajo commented 2 years ago

Yea, I had a similar issue with styletron.

I had to write a bit hacky code to "retarget" the engine so it starts adding styles to the correct document, you should be able to do something very similar since Fela is pretty much identical:

https://github.com/uber/baseweb/blob/master/.ladle/components.tsx#L23-L40

Frankly, I couldn't come up with a more elegant solution other than just reloading the page when turning on/off the iframe. If anyone has some ideas... adoptedStyleSheets looks interesting - it could simplify this code but you'll still need to handle the CSS in JS engine targeting the correct document after the switch. Btw, you can't observe mutations for CSSStyleSheet - so no ad-hoc synchronization is possible.

bensmithett commented 2 years ago

Thanks for the pointer, here's how I adapted that for Fela in a way that lets you switch the frame on/off without needing to reload:

function getDocument(story) {
  const iframe = document.querySelector(`[title='Story ${story}']`)
  return iframe && iframe.contentDocument ? iframe.contentDocument : document
}

export const Provider ({ children, globalState }) => {
  const doc = getDocument(globalState.story)
  const felaRenderer = useMemo(() => createRenderer(), [doc])

  return (
    <RendererProvider renderer={renderer} targetDocument={doc}>
      {children}
    </RendererProvider>
  )
}

This could be nicer without needing to write the custom getDocument...


you'll still need to handle the CSS in JS engine targeting the correct document after the switch

I might be misunderstanding adoptedStylesheets but if the idea is that documents/shadow roots can share live updating stylesheets, you could possibly avoid that. Just always target the root document, then adopt in the iframe when you have an iframe? But it'd only work with constructed stylesheets which probably isn't much good for most people.

tajo commented 2 years ago

Possible to directly pass in the current story document on globalState or something?

I had a version passing it into the Provider at one point. Also, there's probably only a single iframe anyway, so you could just query that.

I might be misunderstanding adoptedStylesheets but if the idea is that documents/shadow roots can share live updating stylesheets

That would be great. I will test it out. That would remove any need to custom setup CSS in JS libs.

tajo commented 2 years ago

Tried to use adoptedStylesheets. It seems you can't grab <style> element from the parent and do iframe.document.adoptedStyleSheets = [styleElement.sheet].

It throws:

Uncaught DOMException: Failed to set the 'adoptedStyleSheets' property on 'Document': Can't adopt non-constructed stylesheets.

The adapted sheet needs to be created/shared through

new CSSStyleSheet()

So that rules out some type of seamless integration on the background. Not sure if most CSS in JS libraries even provide CSSStyleSheet instance directly instead of just adding style DOM nodes in a set container.

Also, this whole thing doesn't work in Safari.

bensmithett commented 2 years ago

Good to know, appreciate you giving it a try 🙏

I'm happy with the workaround. Would you like me to PR a docs update mentioning it or something?

tajo commented 2 years ago

Yea, I think it would nice to document it here with some examples: https://ladle.dev/docs/width

I am also considering to provide some onIframeChange callback passed through the Provider to simplify the user setup.

twhitbeck commented 1 year ago

I believe a project using Chakra UI which uses emotion for styles also suffers from this problem.

tajo commented 1 year ago

I have a solution for this now, should work for all CSS in JS libs without any strange workarounds by users. Will be released in V3.