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

Alternative to className property on View #1318

Closed
RyanThomas73 opened this issue Apr 1, 2019 · 30 comments
Closed

Alternative to className property on View #1318

RyanThomas73 opened this issue Apr 1, 2019 · 30 comments

Comments

@RyanThomas73
Copy link

Is your feature request related to a problem? Please describe.
We use the className property in a few select cases to apply css rules that we otherwise cannot apply through styles. (e.g. ::-webkit-scrollbar rules, etc). After updating to the latest version of react-native-web we now receive continuous warnings stating that the className property is deprecated without providing any documentation for an alternative to achieve the same result.

Describe a solution you'd like
Provide alternative instructions on how we can achieve the same result without using the className property.

Describe alternatives you've considered
We currently patch the react-native-web package after installing to comment out the warning line. While this suppresses the repeated errors it does not provide us a solution for upgrading in the future.

Additional context
react-native: 0.59.2
react-native-web: 0.11.0

@necolas
Copy link
Owner

necolas commented Apr 1, 2019

There is not going to be a replacement API for adding className to primitives. You can still use setNativeProps or the unstable createElement export.

@RyanThomas73
Copy link
Author

RyanThomas73 commented Apr 2, 2019

How would we use setNativeProps to combine our additional className with the one(s) generated by react-native-web. Is there a concrete example of this in the react-native-web documentation?

@steida
Copy link

steida commented Apr 2, 2019

@RyanThomas73 Use setNativeProps inside component did mount.

@necolas
Copy link
Owner

necolas commented Apr 2, 2019

Yes use it like any other instance method. You can read more in the "direct manipulation" docs.

@necolas necolas closed this as completed Apr 2, 2019
@RyanThomas73
Copy link
Author

@steida @necolas Yes but won't using it overwrite the className values set by react-native-web as a result of the style values?

The current use of className property combines class name with the className(s) generated by the style objects.

@necolas
Copy link
Owner

necolas commented Apr 2, 2019

Yes but won't using it overwrite the className values set by react-native-web as a result of the style values?

Oh yes it will. Use a data-* attribute and target that with your CSS. Generally you should migrate away from using external CSS and bring up any use cases that might be worth accommodating

@RyanThomas73
Copy link
Author

@necolas Thanks I'll try using the data-* attribute. I agree we should avoid using external css as much as possible but there are still plenty of cases like the ::-webkit-scrollbar css rules we're using which react-native-web doesn't support.

It's important for us to have a way to use the features that react-native-web either decides not to support or has not yet had time to implement.

@necolas
Copy link
Owner

necolas commented Apr 3, 2019

People need to share relevant use cases (not solutions)

@kopax
Copy link

kopax commented Apr 22, 2020

I want to add a hover on my react-native-paper menu, I'd like to hook a css and target the div, but I can't without getting warning, any solution (I just give you a specific use case)

@TheMightyPenguin
Copy link

TheMightyPenguin commented Jul 9, 2020

@necolas We're doing server side rendering with RNW, but we also need media queries for our desktop application as it's important that the app looks correct on the first render or the markup coming from the server. We need to inject a className to our components to do this, but currently as we need a ref to the component and run setNativeProps on the server to accomplish this, it's impossible for us to get these classNames in our components on the server render and for them to look good on the web on first paint with the current APIs offered.

I'm mentioning this as an use case that we have.

Similar to the points brought up here: #1146 (comment)

@necolas
Copy link
Owner

necolas commented Jul 9, 2020

#1318 (comment)

@TheMightyPenguin
Copy link

@necolas but to set a data-* attribute we'd also need to use setNativeProps right?

@necolas
Copy link
Owner

necolas commented Jul 9, 2020

Forwarding of data-* props is no longer supported. Use dataSet instead. For example, dataSet={{ someName: 1 }}.

https://github.com/necolas/react-native-web/releases/tag/0.13.0

@nandorojo
Copy link
Contributor

Similar to @TheMightyPenguin, I am trying to server-side render RNW using Next.js. If I use Dimensions to conditionally render styles, the server doesn't necessarily match the client's first render, which breaks the app (see vercel/next.js#14469).

I've come up with 2 solutions:

  1. Lazy-load the components, but miss out on SEO benefits, which mostly defeats the purpose SSR.
  2. Use CSS for media queries while on web. I know it's better to use RN directly for styles, but this seems like the only true fix when server-side rendering.

I've relied on styled-components (not styled-components/native) in order to generate these CSS media queries. If the className prop weren't deprecated, this solution would work well.

I know @necolas recommends against using styled-components like that, but it's the easiest way I've found to inject CSS media queries in my web files. styled-components also has good integrations for both React Native and Next.js, making this a really nice DX.

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.


Is this considered a strong enough use case for using the className @necolas? I'm not sure how else to generate CSS media query styles from JS.

If not – is anyone aware of a package that generates CSS modules based on custom selectors, which could also work for server-side rendering/Next.js and RNW?

I'm happy to share minimal code to illustrate if that would help.

@TheMightyPenguin
Copy link

TheMightyPenguin commented Jul 21, 2020

@nandorojo I actually tried to make it work with styled-components, and used the className in a data-class attribute, but that just overcomplicated things as now we needed to modify the CSS we generated on the server to include this selector. This was too complicated, and also it's not the direction React Native Web is moving so in the long run this was going bite us, so we decided to embrace handling responsive styles in JavaScript like RNW recommends.

We ended up doing an approach similar to this one, and when rendering on the server, we use the user-agent header to know what styles to render initially. It's not as precise as using a CSS media querie based on the resolution, but it's working well for us, and it definitely goes more aligned with the principles of RNW and it's easier to implement. Hope this helps!

@nandorojo
Copy link
Contributor

nandorojo commented Jul 21, 2020

@TheMightyPenguin I see, thanks for the heads up. Maybe I could rely on the same solution.

The downside is that for some percentage of users, the user agent will be wrong, so it doesn't feel like a safe workaround. If I rely on screen size on from the server, someone could resize their screen between the time that they first opened the URL and the time the client renders. If they resize to a different breakpoint during that time, the client & server props won't match, and the correct styles won't apply since the Next.js app will break.

I would definitely prefer to keep this all in JS if I could. It makes the most sense, and I agree that it's the right direction for RNW. Unfortunately, the trade off of losing good SEO seems too high. And a solution that doesn't work for every user makes me a bit nervous to put in production.

@TheMightyPenguin
Copy link

@nandorojo is that actually an use case where your users are resizing the page much? In our case it's not that common, I think it's an edge case you shouldn't worry too much about 😉 . If users resize on the client and the page has already rendered, it's fine as styles will be handled in JavaScript, so styles will change nicely. Also on the server you cannot rely on a screen size as you don't have that information, but instead on the user agent string which will look something like this:

Mozilla/5.0 (iPhone; CPU iPhone OS 12_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.0 Mobile/15E148 Safari/604.1

https://deviceatlas.com/blog/list-of-user-agent-strings

So you use that to determine what kind of device the user is using (mobile, tablet, desktop, ...), and then apply styles based on that, and there are also many libraries that parse this string and help you to detect the device.

Regarding SEO, why is that a concern? Rendering the mobile/tablet/desktop version of your markup shouldn't affect SEO if all follow semantic HTML practices, or if you have things like JSON-LD structured data, so I'm not sure you would lose your SEO score.

@nandorojo
Copy link
Contributor

nandorojo commented Jul 21, 2020

@TheMightyPenguin Yeah, I don't think it's a super common situation. It's just an edge case I would like to prevent. I'll keep testing it though. I think you can get the screen width using client-hints actually, but I'll try both that and the user-agent.

The reason I would sacrifice SEO would be if I lazy-load the components and stick with JS styles. React SSR requires that all components from the first client render & the server render have the same props. If they aren't, then it will render whatever the server generated and it won't re-hydrate (see: vercel/next.js#14469). I'll see if I can make it work with user-agent for now, and hopefully I can just avoid edge cases.

@TheMightyPenguin
Copy link

@nandorojo Nice! I didn't know about client-hints, thanks!
Also, you can just render SSR and if props don't match react will just do a full render right? In that case the worst case scenario is that for some users they will have an extra initial render on the client, but I guess you'll have to see how many users does that affect

@nandorojo
Copy link
Contributor

nandorojo commented Jul 21, 2020

Also, you can just render SSR and if props don't match react will just do a full render right?

That's what I thought:

I guess my question is — why does the style prop have to match? Props change in react all the time. Can’t the app just rehydrate the changes when it opens on the client?

Turns out, React requires your server and client to be the same, and if they aren't it will still render whatever the server generated.

What you're attempting to do requires a DOM (window or document to be present) and the server doesn't have one. What you're asking for is to essentially disregard/throw away the SSR generated content (perhaps generating markup and using data for a mobile device) and possibly destroying/recreating it on the client (perhaps regenerating markup and refetching data for a wide-screen desktop monitor). This translates to (possibly) rendering two different apps for each initial application load.

There's more info about that on Google's website: https://developers.google.com/web/updates/2019/02/rendering-on-the-web

In any case, there appears to be a trade-off with deprecating className if you want 1) responsive styles, and 2) server side rendering. If you just use CSS, there is no such trade-off (other than the fact that we want to stay away from CSS.)

If you have to do a full re-render, you're getting "one app for the price of two" performance-wise, as Google puts it.

I guess we can bank on phones and internet getting faster, so maybe this won't be a problem going forward. The downside is that I don't know how to check from the client if the props are different from the server. So if I want to avoid the edge case of client and server not matching for the edge case I described above, it might require a re-render no matter what.

I appreciate your help with this, by the way.

@TheMightyPenguin
Copy link

TheMightyPenguin commented Jul 21, 2020

Yeah, I guess the "double render" is a price we're going to pay in some requests, but I doubt it will be that often, we'll be monitoring our logs to see often we get a props mismatch due to this to see if it's a problem worth solving 👍 For a bit more context, what we do is something like:

// where our app renders in SSR
<DeviceProvider initialDevice={initialDeviceFromUserAgent}>
  <App />
</DeviceProvider>


// some component
const SomeComponent = () => {
  const device = useGetDeviceType();
  
  if (device === 'mobile') {
    return <MobileJsx />
  }

  return <DesktopJsx />
};

At the begining I was skeptic about using a device name instead of a breakpoint, but it's actually quite nice!

And likewise 👍 It's nice talking with others with the same challenges 😄 !

@nandorojo
Copy link
Contributor

nandorojo commented Jul 21, 2020

I see, thanks for sharing.

One thing I did notice just now - while that might work for server side rendering, it doesn't work for Next.js's Static Site Generation. I should have been more clear in my wording earlier for what I'm using.

Static Site Generation can't rely on the user agent since the pages are generated at build time. And so, a forced re-render would be necessary basically every time.

Also, doesn't user agent only tell you the device but not size? What if a desktop browser isn't full screen width?

@nandorojo
Copy link
Contributor

nandorojo commented Jul 21, 2020

Here is my current solution.

The downside is that there is a flash of unstyled components 🤕

const useIsomorphicLayoutEffect = typeof window !== 'undefined' ? useLayoutEffect : useEffect

function useScreenWidth() {
  const [width, setWidth] = useState( 
    Platform.select({ web: 0, default: Dimensions.get('window').width })
  )

  useIsomorphicLayoutEffect(() => {
    if (Platform.OS === 'web') { 
      const { width } = Dimensions.get('window')
      if (width >= breakpoints[0]) { 
        setWidth(width)
      }
    }
  }, [])

  useEffect(() => {
    const listener = (event) => {
      setWidth(event.window.width)
    }
    Dimensions.addEventListener('change', listener)
    return () => {
      Dimensions.removeEventListener('change', listener)
    }
  }, [])

  return width
}

I don't know the performance implications of useLayoutEffect running on mount, but it seems to be working for Next.js.

In an effort to not spam people who are subscribed to this issue, I'll be moving discussion to nandorojo/dripsy#1. Please follow up there if you are having issues or have solved this problem in another way.

@TheMightyPenguin
Copy link

TheMightyPenguin commented Jul 21, 2020

@nandorojo right, with static site generation the only way would be with CSS media queries 😢

Also, doesn't user agent only tell you the device but not size? What if a desktop browser isn't full screen width?

Yeah, you're right, it's not a perfect solution but is a place to start!

@steida
Copy link

steida commented Jul 21, 2020

Check https://github.com/artsy/fresnel

@nandorojo
Copy link
Contributor

@steida thanks so much for sharing. Looks like I'll be using fresnel.

@nandorojo
Copy link
Contributor

An update in case anyone is wondering: I removed SSR support for Dripsy and my app lol. Trying to maintain media queries with CSS hacks just wasn't worth separating from RNW.

@TheMightyPenguin
Copy link

@nandorojo totally agree. In our case we rely on the user agent on the server side, it has worked well so far, but yeah we tried going down that route and it was painful.

@KeitelDOG
Copy link

@nandorojo I stopped using Fresnel with Next JS + Native Web after too much struggle. But then I found another one working like a charm, more like a media queries within styles "react-native-media-query": "^1.0.9",. Very amazing.

@KeitelDOG
Copy link

KeitelDOG commented Dec 3, 2021

@necolas I use React Native Paper, a Material UI alternative for React Native.

In Material UI, for each component and each HTML tag that's part of a component like TextView, Button, Avatar, div, p, input, span etc. they injected special classes like Mui-TextView-root etc. But in React Native Paper, they don't, so that small hacks on the look is no more achievable.

While React Native Paper works normally for most components, some pose some very light problem that can be solved with 1 CSS Rule. But we must be able to target a Component with class to rule it in external CSS.

We know we should stay away, but hacking components with CSS in Next JS or Web side doesn't have to be mixed up with React Native if it already display well. Also, instead of asking React Native Paper staff of other packages staff to fix all Single CSS rule problem, we could work around on them all with classes.

Maybe the data- standard can work well in some case, but React Native Paper can put it anywhere from most Outer HTML Tag or most Inner HTML Tag, or not at all, and we loose the cascading advantage.

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

No branches or pull requests

7 participants