Open kentcdodds opened 2 years ago
Hm, I'm going to be extremely honest here — I haven't thought about the grouping and ordering in the internal sheet code in months, so my thoughts on this may not be entirely accurate.
The groups are registered as components are created; that's by design, since we're registering the order in which they were created rather than the order of their first render call. (or first effect for that matter)
So, when these IDs become inconsistent and change, that's often an indication that you aren't dealing with the same instances of components anymore and they've been recreated, or the modules have been replaced.
The reason that's relevant is that if they were to just be reset, there's no way to actually get back to a state where they're instantiated in creation order, unless the components are actually fully recreated.
Now, again, I haven't thought about this in a while but some time ago I actually wanted to figure out some more tricks to make componentId
for SSR obsolete, and make this overall more consistent. I simply don't have much time allocated for myself on styled-components anymore
What's interesting here though is that we may want to first check how these IDs are changing. Specifically, if they all change at the same time (i.e. all components are recreated, is the only difference the minimum number now starting at the last maximum ID +1?). If so then we'd only have to ensure that for SSR we always adjust all of them by subtracting the minimum.
Makes sense I think. The fact is that we don't really need to worry or care about order inconsistencies for the server render because there are no re-renders on the server. So we just want to simulate the same experience on the server as the client has for its initial render, which means: no existing groups.
I think exposing this function is all that's needed. Getting rid of the groups seems more dangerous and would require more time. I think exposing this function would be a great stepping stone and unblock folks who want to do SSR without babel.
Well, it depends on whether any rehydration happens on the client-side because currently we basically use server-ordering to initialize client-ordering. Admittedly, we can maybe reduce that somehow, but the problem is then still that you need to associate which components are the same on client and server side, hence the Babel plugin's component IDs. However, one idea I had was to associate them using heuristics and render ordering but then to leave the group IDs independent in both places. Either way, it's a little tricky
Can we not assume that the first client render will be the same as the first server render? I think that's a safe assumption since otherwise you get a hydration warning.
So, to just write this down, I think what I was thinking about last, in terms of what could work was to change the sheet logic to this:
useId
outputuseId
outputuseId
and deletes & replaces styles in the SSR styles before mounting new stylesThis would mean that the client- and server-styles are completely independent from one another but we can still associate them and delete server-styles as needed at the correct time.
As long as we then use useInsertionEffect
, in theory (!), there should never be a tick where styles are conflicting.
Sounds like that could work for us! I'll update the issue title to describe what I'm really looking for rather than the specific implementation.
I found additional problems with this approach;
The first problem is I'd love to avoid work by not having to run "rehydration" of all styled-components' related styles beforehand. This obviously excludes streamed <style>
tags; those will still have to be removed from the DOM and put into a shared <style>
tag. However, it'd be great to not do too much work (e.g. know why group of styles belongs to which component until that component actually mounts)
This isn't possible however.
There's two things preventing the useId
approach entirely today:
useId
available. It becomes a hen and egg problem. We cannot associate an SSR style group with a component until it's mounted, and we cannot mount a component (or rather; rehydrate it) until we have associated an SSR style group (and order ID) with itcomponentId
as it's possible to create styles based on a component ID and having this ID be unstable means that we potentially have it change which causes SSR mismatches anyway.There are several complications here but basically, we still need a stable component ID, which inherently means that useId
cannot work. If we found a way to make it work then it'll always run into issues where creation order interferes with it. Again, groups of components may be created in a changing order (due to chunking/bundling changes), which must always work and be alright, however we cannot reconcile groups and orders dynamically during rendering.
If we tried to remove an SSR'd style group one by one during rendering then we may run into issues where we're in a "mixed state" of having SSR styles and non-SSR styles on page at the same time. Let's say two chunks exist and they are initialised in the reverse order on the client than on the server (which may be valid!), then one chunk may have IDs 2,3,4
on the client and the second chunk has 5,6,7
while on the server-side this may have been the opposite.
If we then go through the SSR rehydration and only try to reconcile this during rendering, then even if we can tell that the client's component #3
is the server's component #6
, and set component #3
to become component#6
then we cannot reconcile the resulting conflict of the client's already existing component#6
. Swapping the IDs isn't possible because the two chunks individually may be in a different unique order.
Alternatively, we also cannot just create a separate style sheet for the client-side. If we consider that rehydration can be progressive and partial, if all SSR styles are listed out (i.e. styles for 2,3,4,5,6,7
) then we'd always list client styles afterwards (or before; i.e. as 2,3,4
), which would mean that, if rehydration was still partial temporarily, that styles would now be overriding each other in an incorrect order (e.g. 2,3,4,5,6,7
then 2,3,4
), which is exactly what we're trying to prevent.
This is because, while ordering is consistent independently on the server- and client- and consistent, we cannot guarantee that partial rehydration doesn't stop at a different point.
So, all these factors mean that we still need component IDs for multiple reasons.
We could probably do better with keeping component IDs stable, i.e. with a rules hash, but that'd likely make rehydration even more expensive. At this point though, it's hard for me to tell what a good alternative would be.
Okay, quick follow-up. I may not be 100% correct about this. We could simply ignore the differences and let groups be used by both rehydrated and non-rehydrated styles at the same time. As long as they're vaguely similar (which is the case if they both start at the same number) they'll be in a consistent order, and only some sections may be used for two different sets of components each at the same time. As we'd rehydrate we can then eventually maybe delete all of the server-side ones or even leave them intact.
That's not really the best solution but it's one possible solution 😄
That doesn't quite address the issue of us using component IDs as classnames inside CSS though, so either way, it seems hard to get rid of them entirely
I'm fine with whatever you decide. Right now, the styled-components example on Remix works. It just has a hydration warning during development after the first page is requested. It's annoying, but it works. I believe that what you're proposing could resolve this issue.
Is there a way we could pass in the className that gets used directly for each styled component? That's another way we could ensure the same class names on the client and server. Something like:
const component = styled.div.withConfig({className: 'component'})`
I know that there's already a displayName
prop, but it appears that ids still get generated along side it. If we could define what the full class name is that the styles attach to, then we could guarantee consistency.
I know this is more manual for the user, but it also has the added benefit of making consistent className selectors for extensions
creating remix scretch project, and getting this kind of below warning message initially really hurts the developer learning curve and interest.
react-dom.development.js:67 Warning: Expected server HTML to contain a matching
in .
This is why I moved to svelte from remix.
@kentcdodds @kitten Not sure where this was left? Can anyone share any updates? We've established a migration path to more to Remix, and keep our custom react ui to use styled-components, however, seeing those dev errors gives us pause. We don't want to bring back babel-plugin-styled-components hydration issues we saw in the past.
Does anyone have a sense on if these is just "dev" errors/warnings that don't physically impact anything or are they impacting production environments?
As a workaround, you can use patch-package
to export the resetGroupIds
in GroupIDAllocator
and do what I suggest in my opening comment and you'll be in business. It would be a pretty low-risk thing to do I think since it's a very simple change.
Thanks @kentcdodds ! I'll try that.
I have a slightly related issue with remix+styled components where in production the styles from styled-components are missing completely => https://github.com/remix-run/examples/issues/136
Leaving it here since people might have the same issue but land on this one
As a workaround, you can use
patch-package
to export theresetGroupIds
inGroupIDAllocator
and do what I suggest in my opening comment and you'll be in business. It would be a pretty low-risk thing to do I think since it's a very simple change.
Looks like resetGroupIds
is intended for testing and dead code eliminated from the release bundle. So patching isn't an option. The only other option is to fork, export & build.
@jmurzy Gah! Back to looking into the fix for this.
@kentcdodds Can you clearify what you say is working vs not with this issue?
Our example is that the remix site loads up perfectly fine with all the styles on first load however, if we refresh (even with JS disabled) the second load the hashes get all misaligned and thus the styles are broken.
Is this the same issue?
Also, I've found a good deal of issues/posts around using babel-plugin-styled-components's ssr: true
and namespace
options to fix this issue in custom libs. It seems to have no affect for me.
https://styled-components.com/docs/tooling#serverside-rendering
It's been a long time and I don't use styled-components. I was just trying to help others. I'm afraid I don't have any time to remind myself what this issue is about. Sorry.
Nw @kentcdodds I know how it is.
However, what may still be relevant is that Styled-Components, as is, still does not work with Remix, or any other SSR without the aid of babel-plugin-styled-components
which itself is not supported in Remix since it doesnt seem we have access to plugins or modifying esbuild config.
As a comparison, Next.js allows you to control webpack option values from a configuration file (next.config.js) so they can implement the needed plugins.
This limitation seems to be leading some remix users to implement workarounds using a custom module to override esbuild.
Also confused as this has anything to do with the issue. https://github.com/remix-run/remix/discussions/5244#discussioncomment-4832036
We're VERY much wanting to move to Remix but need to slowly migrate away from styled-components, and not cold-turkey cut it off. So we need a migration path inside of Remix.
The patch-package
process described above worked fine for me. I'm afraid this is just not a priority for me. I'm no longer working at Remix either so spending time helping people adopt Remix isn't a priority for me either. Sorry.
Makes sense @kentcdodds, no worries. Can you describe how you got patchpackage to work given what @jmurzy says above... about all the "resetGroupIds" code being stripped out of the npm module as it looks like they only use it in dev?
It's possible I remember wrong and I actually never got it working 😵💫 sorry I can't be more helpful. Feels like a fork may be in order. This package isn't super active, especially if you're planning on migrating away.
I'm trying to find a good way to server render without needing to use the babel plugin. I've noticed that in my example I'm working on that the first server render hydrates without issue, but subsequent server renders have a different group number for the stylesheet and that changes the hash which results in hydration errors. Here's what I'm doing to get this:
So all I would need is for the
resetGroupIds
inGroupIDAllocator
to be exported and then I could do this:Would this be possible?