nandorojo / dripsy

🍷 Responsive, unstyled UI primitives for React Native + Web.
https://dripsy.xyz
MIT License
2.05k stars 82 forks source link

Dropping SSR support #100

Closed nandorojo closed 3 years ago

nandorojo commented 3 years ago

I'm considering dropping SSR support. It's built on top of many hacks, all of which are entirely circumventing the recommendations of react-native-web.

I've elaborated on this problem at length on many issues and PRs in the past.

If you need Dripsy in production with SSR support, feel free to mention it here.

At the moment, supporting SSR means the web API is complex, kind of confusing, and adds more DOM nodes.

I've been using Dripsy with SSR "disabled" (i.e. we return null at the root of the app and then setMounted(true)) on the beatgig.com. No issues for us with performance.

cmaycumber commented 3 years ago

I'm in favor of this.

I think if there's better support for another method down the line that's in line with react-native-web it can always be added.

nandorojo commented 3 years ago

I'm thinking we add an ssr option to the provider. If true, then it'll handle blocking rendering on web for you until it's mounted. You could pass a Placeholder component too.

nandorojo commented 3 years ago

This seems similar to the PWA/Twitter style.

nandorojo commented 3 years ago

I thought this was a good piece explaining why SSR doesn't work with RNW.

https://lihautan.com/hydrating-text-content/

React's hydration expects the initial render to match what's on the server. However, if we're using JS Dimensions, this isn't possible.

nandorojo commented 3 years ago

This also means that custom breakpoints should be easy to implement, since we don't need to use them with Fresnel.

nandorojo commented 3 years ago

I'm working on this update now. It'll be a major version bump.

The only "breaking" changes from the user's standpoint:

All other changes are under the hood.

This will automatically enable custom breakpoints.

In case we, for some reason, change our minds, I'm putting this behind an internal SUPPORT_FRESNEL_SSR boolean for now.

nandorojo commented 3 years ago

This also opens the door to a lot of additions. We can now allow components to pass custom props to styled. Or a useSx hook. I'm excited for the doors it opens.

nandorojo commented 3 years ago

If you could test this, it would be great.

yarn add dripsy@canary

That should install 2.0.0-alpha.0

nandorojo commented 3 years ago

I added notes for making this work well for SEO at the PR: https://github.com/nandorojo/dripsy/pull/101

nandorojo commented 3 years ago

I wish I'd done this sooner. It is simply amazing. So many doors are suddenly open for responsive styles. No more hacks 🔥

cmaycumber commented 3 years ago

Yeah, I'm excited. I think if I need anything with responsive styling I'm just going to fall back to a theme-ui component on web which is working great for me.

nandorojo commented 3 years ago

Yeah, I've been doing the same. Do you mean the one that I shared a while ago?

nandorojo commented 3 years ago

I'm probably going to just stick to the normal API and block the SSR. Maybe we could add docs for the theme-ui Div component if someone really needs to render on the server. But I'll consider it officially unsupported as a userland solution

cmaycumber commented 3 years ago

Yeah, I've been doing the same. Do you mean the one that I shared a while ago?

I've used that one for inspiration with a few tweaks and am also doing it for text bc I find that usually on layout component and text are the ones that I really need to use responsive styling for.

I'm probably going to just stick to the normal API and block the SSR. Maybe we could add docs for the theme-ui Div component if someone really needs to render on the server. But I'll consider it officially unsupported as a userland solution

Nice, I might do the same down the line once I get a chance to look through my app bit. Quick question do you still see a benefit for using Next Js over just base expo web without SSR support. The first thing that comes to mind is code-splitting so pages have smaller bundle sizes but I'm curious about your take.

nandorojo commented 3 years ago

Yeah I think next is great. Code splitting and navigation. Also I need it for static props in order to generate meta tags for different URLs.

For instance, try sending yourself beatgig.com/@djkhaled

Notice the image and text that shows in the preview, next is really useful for that

nandorojo commented 3 years ago

Also next is just the cutting edge of React web, I like being close to that

cmaycumber commented 3 years ago

Cool thanks for the input. I definitely like the flexibility it's giving me, it feels like it's really possible to support almost every use case that I need to.

nandorojo commented 3 years ago

Yeah agreed. The finally frontier for me is navigation. I solved a lot of it with expo-next-react-navigation. But the missing piece was stacks / modals based on route.

I've finally solved that internally, but I haven't uploaded the code yet.

Context: https://twitter.com/FernandoTheRojo/status/1388572885833375756?s=20

image

I render a stack per-Next.js page. Then I have a single stack on native with native-stack, and I give the next pages & modals the same names as the screens on native. I'm turning it all into a consolidated API, it's nice.

nandorojo commented 3 years ago

lol GitHub messed up the URL I sent for beatgig, just fixed it

cmaycumber commented 3 years ago

Yeah agreed. The finally frontier for me is navigation. I solved a lot of it with expo-next-react-navigation. But the missing piece was stacks / modals based on route.

I've finally solved that internally, but I haven't uploaded the code yet.

Context: https://twitter.com/FernandoTheRojo/status/1388572885833375756?s=20

I render a stack per-Next.js page. Then I have a single stack on native with native-stack, and I give the next pages & modals the same names as the screens on native. I'm turning it all into a consolidated API, it's nice.

Sweeeeet. Can't wait for this to drop. I'll be one of the first to use it! Does this still use expo-react-next-navigation? Or is it a separate package?

nandorojo commented 3 years ago

Yes it'll use that package still. I'll probably put it inside of it. I might make a moti version too, or just show a code sample with Moti.

nandorojo commented 3 years ago

Removed every instance of webContainerSx from our app. So satisfying.

nandorojo commented 3 years ago

yarn add dripsy, v2.0.1 removes fresnel and SSR.

redbar0n commented 3 years ago

Do you know what SEO implications this has? I read the SEO recommendations you referenced, and looked a bit at next-seo, but it remains unclear what, if any, SEO impact it will likely have...?

Have you measured the before/after impact on beatgig.com search index ranking, for instance with Fetch as Google and measuring with the Google Web Vitals ?

It would be interesting to learn the results, to get a feel for the tradeoff.

redbar0n commented 3 years ago

I need Dripsy in production with SSR support.

nandorojo commented 3 years ago

@redbar0n you can still use 1.x for this. Unfortunately, maintaining SSR development wise simply isn't worth it. It's the equivalent of maintaining a fork of react-native-web, with style inconsistencies. While I share the concerns about SEO, etc., the problem lies with react-native-web deprecating className, not Dripsy. There are some hacks you could use, if you want, but I don't recommend them.

If you really need SSR support for your render code, you'd need to 1) not use React Native Web, or 2) not use responsive styles. There is no alternative without hacks that are hard to maintain.

nandorojo commented 3 years ago

If it helps, Twitter doesn't SSR besides meta tags, judging by the fact that opening Twitter shows a loading spinner first

nandorojo commented 3 years ago

I've written about this on many issues and PRs, but this is where I tried my hardest with RNW: https://github.com/necolas/react-native-web/issues/1318

nandorojo commented 3 years ago

@redbar0n as you mentioned on the Magnus repo, RNW is totally against SSR: https://github.com/necolas/react-native-web/issues/1688#issuecomment-665939373

As for the effect on BeatGig's SEO, here is the result from Google's mobile website parser:

image

It successfully loads in the HTML too, you can see by clicking the HTML tab: https://search.google.com/test/mobile-friendly?id=DJQI7CPIa8_RVmHkLDI_SA

The only "warnings" are console.log warnings about reanimated 1 deprecations.

For context, beatgig.com is using the latest version of Dripsy with SSR disabled. It also uses Expo + Next.js.

redbar0n commented 3 years ago

Thanks, and I understand the concern and your decision.

This was also helpful for me to understand the tradeoffs in dealing with hydration, CSS, SSR, Fresnel etc., like the other link you shared previously re. hydration:

https://aboutmonica.com/blog/server-side-rendering-react-hydration-best-practices#summary

I saw in that issue you mentioned, you said:

The unfortunate part, of course, is that styled-components relies on className exclusively, and doesn't allow you to use a data-*/dataSet.X prop as a CSS selector.

What if styled-components or some similar library allowed dataSet? Could that circumvent the whole issue? Given that styling libraries are quite abundant and change frequently, it might be a hope that there is or will be one out there that suits. It would surely be a shame if this were the only thing to block the awesome combo of RNW+ NextJS from being fully realised.

nandorojo commented 3 years ago

You could try. But at this point I've come around to agree with Nicolas. Let's stay in JS-land and build apps that are self-contained in React component styles. They should be consistent across platforms and not have CSS side effects. So I have given up on the SSR train when it comes to RNW. Ever since I stopped, my coding speed has been much better, and now I can comfortably use crucial hooks like useWindowDimensions, etc to style content.

redbar0n commented 3 years ago

Actually, come to think of it, I don't actually need SSR support for the user, it's simply for SEO purposes (search engine crawlers).

I was inspired by the tradeoff this guy made, although he used the more complicated Rendertron instead of NextJS to achieve it, it seems.

So, if something (Dripsy?) simply detected the Googlebot User Agent, and used NextJS to SSR a (non-responsive) mobile version of the webpage, it'd be a good tradeoff in most cases, I think.

Then users could have the primary, fully responsive, CSR'ed version (which likely would be fast enough). Without the app having to sacrifice SEO in general.

Would that work?

nandorojo commented 3 years ago

I'm only using static generation, not SSR, which can't conditionally render pages anyway.

Maybe your solution would work. You'd have to set the window dimensions yourself somehow, and Dripsy would read from that if it's a crawler.

Just FYI, this isn't something I find a priority at the moment. I spent too much time optimizing for this and ultimately saw no benefit to our site for it. But if you can come up with a solution that's easy to drop in, I'm open to it.

redbar0n commented 3 years ago

At the moment, supporting SSR means the web API is complex, kind of confusing, and adds more DOM nodes.

With more DOM nodes, I assume you mean <Responsive> wrapper components emitting co-located <style> tags to override global styles, like what Horacio Herrera did in his "React Native web + Responsive styles + Server-side rendering Case study" video ? Was this the approach you also took?

nandorojo commented 3 years ago

I meant that every time we used responsive styles, we used fresnel, which added an extra DOM node per-breakpoint.

redbar0n commented 3 years ago

I've dug a bit into this. Likely, this is all familiar knowledge to you, but I'm writing it down nonetheless, just in case it might be useful for you, others, or me, when tackling this issue in the future.

The overall goal still being: How to combine React Native Web (RNW) + Responsive styles (media queries) + NextJS Server-Side Rendering (SSR), to get SEO on the web.

But this time, not even sacrificing SSR for end users like in my previous suggestion. Because it might be problematic long-term to SSR only for the Googlebot, since it’s effectively serving the Googlebot something else than what the users get.. Google might crack down on it, since it could be abused.

Here's what I've found:

If you are worried about maintaining a fork of RNW with potential style inconsistencies, then react-native-media-query is already that fork. It attempts to do media queries inside RNW's StyleSheet.

But to be automatically future-proof, I would rather add the media queries in a thin wrapper around StyleSheet. Taking all the CSS that RNW outputs on web, and put it inside one or more media query blocks. Media queries were designed to wrap big blocks of CSS anyway.

The normal RNW output from StyleSheet:

<style>
  .rn-1mnahxq { margin-top: 10px; }
  .rn-61z16t { margin-right: 10px; }
  .rn-p1pxzi { margin-bottom: 10px; }
  .rn-11wrixw { margin-left: 10px; }
</style>

https://necolas.github.io/react-native-web/docs/styling/#how-it-works

The proposed Dripsy output from the wrapped StyleSheet, with one media query per designated viewport breakpoint:

<style>
@media screen and (min-width: 1024px){
  .rn-1mnahxq { margin-top: 10px; }
  .rn-61z16t { margin-right: 10px; }
  .rn-p1pxzi { margin-bottom: 10px; }
  .rn-11wrixw { margin-left: 10px; }
  // …
  // and all the rest of the complete set of styles, whether component or atomic styles
}
@media screen and (min-width: 480px){
  .rn-1mnahxq { margin-top: 5px; }
  .rn-61z16t { margin-right: 5px; }
  .rn-p1pxzi { margin-bottom: 5px; }
  .rn-11wrixw { margin-left: 5px; }
  // …
  // and all the rest of the complete set of the styles, whether component or atomic styles
}
</style>

It seems like it is a breeze to get the stylesheet that RNW outputs, by using:

import styleResolver from ‘../StyleSheet/styleResolver’;
const sheet = styleResolver.getStyleSheet();
const content = sheet.textContent(); // to get the atomic CSS for the rules that have been resolved.

From this RNW code.

It would mean that the media queries would be output by Dripsy itself. But the user shouldn't need to care about media queries directly anyway. It's more user-friendly to have a Dripsy separate API for responsivity, that allows users to declare the necessary screen sizes and breakpoints for the media queries.

It would mean to avoid styled-components, and take control of CSS output directly from Dripsy. But as I’m sure you’ve discovered, using styled-components with React Native Web + responsivity, is problematic, to say the least.

To apply styles conditionally in JS, based on window dimensions (client-side), then instead of the JS Dimensions API, you could use:

if (window.matchMedia(“(max-width: {getBreakpoints().width} )”).matches)

It can be run only client-side, by using typeof window === ‘undefined’.

Wrapped in a hook it could be exposed as:

if (clientWindowSize(“medium”)) { /* then apply this style */ }

(It seems matchMedia is also what Fresnel uses in their <Media> components to prevent React from rendering unnecessary breakpoints client-side.) You could either put this into a replacement for RNW’s useWindowDimensions() when on web, or expose it in another hook, perhaps better named. Since media queries are designed so you shouldn't need to know the specific window dimensions.

To avoid React's hydrated HTML not matching what was SSR'ed, then you could use a vanilla JS DOM selector to put the viewport dimensions into a <div id=“breakpoints”> outside of the React <App> tree. Breakpoints could be user specified/pre-determined, or server-determined if using Fresnel. Props can be same client-side and server-side, and the component logic is simple: If the <div id=“breakpoints”> is there, use it, if not, then set it. If using Fresnel, this way, the client is ensured to use the same media query breakpoints as determined by Fresnel server-side.

Custom breakpoints. Developers could statically set custom breakpoints with Dripsy, which I presume Dripsy could forward to Fresnel custom breakpoints by using its createMedia(). But the breakpoints refer to, and is applied in, the DOM/markup with the <Media> wrapper components that Fresnel uses. So I think you currently cannot escape exposing those or an equivalent wrapper component, like SSRMediaQuery or SSRComponent, to the developer. As of now. But this might change, if Fresnel exposes a useMedia() hook… (which might also remove the extra DOM nodes Fresnel adds).

I see 3 general ways for SSR with RNW:

But common for all ways to get SSR, is to avoid the JS Dimensions API in favor of media queries, to also get responsivity.

I understand that this is not a priority of yours at the moment. I just thought that it would be good to have these ideas out there, in case there is something new here. If not, it could hopefully serve as a (linkable) summary to others going down the same path.

nandorojo commented 3 years ago

Thanks for the in-depth discussion here. I did read it closely. Hopefully there will be a time when react native can work with SSR without any hacks that circumvent it. This is doubly interesting with the announcement of React 18 and all of its innovations around SSR / Concurrent Mode.