-
Notifications
You must be signed in to change notification settings - Fork 246
feat: styled-components & streaming #588
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
base: main
Are you sure you want to change the base?
Conversation
…o vite and streaming responses
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a single comment.
(I'm just a passerby who is also interested in using styled-components with remix, ty!)
chunk instanceof Uint8Array | ||
? decoder.decode(chunk, { stream: true }) | ||
: chunk.toString(encoding || "utf8"); | ||
renderedHtml = renderedHtml.replace( | ||
"__STYLES__", | ||
styleSheet.getStyleTags() | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it possible for the __STYLES__
string to be truncated across two different chunks?
In that case, the styles would not get replaced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in theory this could be true, you're right. unfortunately i don't understand node streaming and how the chunks are cut in depth. however, we have been using this approach in production for 2 weeks (first with remix v2 and now with react-router v7) and have never noticed this side effect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my own project, I ended up using
Which handles chunk boundaries correctly, but I'd lean against including that additional dependency in this pull request.
I do think the edge case should be handled correctly, but I don't have any alternative suggestions as to how, other than reimplementing similar logic. But that seems like overkill maybe 🤷♂️
Introduces:
renderToPipeableStream
instead ofrenderToString
Streaming & Styled Components
Hello Remix community,
We've been working extensively with Remix at our company for about a year now. However, our legacy stack still requires (and likely will continue to require) support for styled-components.
Over the past few days, I’ve been working on enabling all the v3 future flags in our project to prepare for the transition to React Router v7. One challenge I ran into is that the
v3_singleFetch
flag explicitly requires switching fromrenderToString
torenderToPipeableStream
.With the help of this comment, I arrived at a solution that I’d like to share and propose here. Unfortunately, using remix-island wasn't a viable option for us—we encountered issues with error boundaries, Remix's
meta
API, and complications with our CDN setup that made integration difficult.With this implementation, everything appears to be working smoothly on our side. I have no prior experience with Node.js streaming, so I’m very happy to receive any feedback or suggestions you might have.
Thanks!
Open topics
_boundary
route as described in the README, but I couldn't figure out what exactly needs to be changed. Maybe someone knows how to prevent the app from unmounting on every change when using styled-components in watch mode.