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

Support Web Streams #2781

Open
Industrial opened this issue Jun 9, 2022 · 12 comments
Open

Support Web Streams #2781

Industrial opened this issue Jun 9, 2022 · 12 comments

Comments

@Industrial
Copy link

The problem

Emotion supports Node Streams but not Web Streams

Proposed solution

Add a method renderStylesToTransformStream() (https://developer.mozilla.org/en-US/docs/Web/API/TransformStream)

This allows you to .pipeTo(transformStream.writable) and to read from transformStream.readable e.g. response.body = transformStream.readable.

Alternative solutions

Waiting for the implementation of node helper functions in deno so that I can create a wrapper that converts a web stream to node, pipe that to the emotion node stream and cast the result back to a web stream. I have a ticket open for that here: denoland/deno_std#2310

Additional context

Web Streams are the new hot shit and Deno supports these natively. I've written a Streaming Server Side Rendering framework for Deno called Reflex that uses suspense rendering and I'd like to use MaterialUI which uses Emotion.

@Brams-s
Copy link

Brams-s commented Jun 30, 2022

This would be valuable to support the new Next.js Edge Runtime & Deno's Fresh Framework as well as other Web API based runtimes.

@Andarist
Copy link
Member

I would be happy to discuss this feature with somebody willing to implement this - my time capacity is limited and I can't afford lately to spend too much time on the OSS. I can design this feature and review the implementation though.

@ItsWendell
Copy link
Contributor

ItsWendell commented Jun 30, 2022

I added a similar ticket, this time to fix how @emotion/cache works with the Next.js Edge Runtime, @Andarist I would love to have a little chat to see how I could contribute to making these Web APIs and runtimes work!

#2801

@nicksrandall
Copy link
Contributor

@Andarist Now that #2819 is almost finished, I may have capacity to take on this over the next month or so. That said, I wouldn't even know where to start so I'd definitely need your guidance.

@Andarist
Copy link
Member

Sure thing - I'm happy to discuss this whenever you find time for this.

@cyco130
Copy link

cyco130 commented Jul 29, 2022

I took a look at this recently to judge the amount of effort needed and to see if I could be of any help. It seems the biggest challenge is to port (or find/create a replacement for) html-tokenize for web streams. The rest seems more or less trivial: Wrap the SSR stream in a TransformStream and generate and emit newly added styles but only just before an "open tag" token is encountered. Maybe an open tag detector could be easier to implement than a full tokenizer (I doubt it though).

Am I on the right track here?

@Andarist
Copy link
Member

Using a strategy close to the current one (based on something like html-tokenize) is an option. However, I wonder if we couldn't just "insert" styles "directly" to a cache from within render and then just flush~ whatever got inserted in between the last call to transform~ and the current one as <style/> element(s).

I'm not sure if there is an actual benefit of reverse-engineering what styles were inserted from the HTML string if we can control the insertion call.

Apart from injecting <style/> we also might need to inject <script/> because we need to move asap the injected <style/> elements away from what React will try to rehydrate

@cyco130
Copy link

cyco130 commented Jul 29, 2022

Yes, I also thought it was kind of inefficient to parse the HTML stream instead of remembering what styles are rendered. I would definitely prefer an implementation like you described but that requires much deeper knowledge of the codebase, which I don't have :)

Apart from injecting <style/> we also might need to inject <script/> because we need to move asap the injected <style/> elements away from what React will try to rehydrate

It makes sense. I couldn't find where createRenderStylesToNodeStream does that though?

@cyco130
Copy link

cyco130 commented Jul 29, 2022

Also, I inject script tags to serialize suspense cache and React doesn't seem to complain. Maybe the hydration engine just skips styles and scripts? This comment seems to imply so.

@Andarist
Copy link
Member

Yes, I also thought it was kind of inefficient to parse the HTML stream instead of remembering what styles are rendered. I would definitely prefer an implementation like you described but that requires much deeper knowledge of the codebase, which I don't have :)

Overall I think that it would be a fairly isolated change - I don't have a good grasp of what it would look like and if it's a preferred approach. To judge this I'd have to see a prototype of the solution. In general, I think that we should attempt to conditionally (based on a flag or something) do things differently in insertStyles, we should just put stuff into some buffer on it (+ probably still put the inserted ids in the cache.inserted). Potentially the code that today uses the insertStyles should also be adjusted. Note that there are just a couple of places where insertStyles is used and at the same time all of them should have roughly the same code structure, so figuring it out for a single call site should be enough as other call sites would have to be adjusted in the very same way. Given that each server request should always use its own unique cache the idea seems to be feasible

It makes sense. I couldn't find where createRenderStylesToNodeStream does that though?

It currently isn't - but it probably should be.

Also, I inject script tags to serialize suspense cache and React doesn't seem to complain. Maybe the hydration engine just skips styles and scripts? reactwg/react-18#110 (reply in thread) seems to imply so.

I'm not entirely sure how to best interpret this comment. I think it's best to just experiment with this - perhaps you are right. This in turn would mean that we'd only have to add a script at the very end of the stream.

@cyco130
Copy link

cyco130 commented Jul 29, 2022

Thanks for the insights, I'll keep looking into it and experimenting as time permits.

@cyco130
Copy link

cyco130 commented Mar 8, 2023

I have a working implementation!

It uses Mantine's server but it works with plain Emotion server too. Unlike Emotion's current Node stream implementation, it also injects a script to remove the style (and the script itself) out of the way, otherwise you get hydration errors if a suspense boundary causes new styles to be injected. It also works on Deno and worker runtimes with some bundler magic to strip Node-only deps (html-tokenizer and multipipe).

Am I on the right track? If so, we can discuss a strategy to extract it into a PR here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants