-
Notifications
You must be signed in to change notification settings - Fork 76
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
Doesn't work with Next.js #1
Comments
Related: vercel/next.js#14469 |
Solved |
Reopening since the solution relies on I'm stuck between trying to find a way to add CSS breakpoints on web to allow for server-side rendered, responsive styles. Since Next.js is likely the main use case (other than maybe gatsby...?) I think a next-specific solution could work. Maybe something like https://github.com/absolvent/react-responsive-next could be the answer. Basically, if someone wants SSR, this lib should support that by using CSS media queries. The only problem is that we need a custom selector (i.e. not |
Could a reworked version of this: https://github.com/hesyifei/emotion-native-media-query which relies on emotion be a possible solution? I'm not sure the tradeoff of using another js-in-css library like emotion, but it would save us dependency wise being that theme-ui already relies on it. |
That's a really interesting package. The one downside is that it's basically re-creating RNW on the fly, which is a really hard thing to maintain. I've had a discussion about this on the RNW issue related to It seems like the future of RN will be styling strictly in JS, so in an effort to keep this library future-proof, I think my original solution (which is currently in this library) won't work. Instead, we need a solution that doesn't use any CSS I tried this in a next.js app, and it seems to be working: // work-around to use useLayoutEffect on SSR
// this will only run on the client
const useIsomorphicLayoutEffect = typeof window !== 'undefined' ? useLayoutEffect : useEffect
function useScreenWidth() {
const [width, setWidth] = useState(
// on web, we set this to 0
// this ensures that our initial render on SSR and the client always has the same props
// which silences the error
Platform.select({ web: 0, default: Dimensions.get('window').width })
)
useIsomorphicLayoutEffect(() => {
// this runs before the component's first paint
if (Platform.OS === 'web') {
// update the width in useLayoutEffect
const { width } = Dimensions.get('window')
if (width >= breakpoints[0]) {
// we'd get the breakpoints variable somewhere
setWidth(width)
}
}
}, [])
useEffect(() => {
const listener = (event) => {
setWidth(event.window.width)
}
Dimensions.addEventListener('change', listener)
return () => {
Dimensions.removeEventListener('change', listener)
}
}, [])
return width
} This would replace the The downside trade-off is that we are re-rendering every component on its first client-side paint. I'm not sure what the performance implications of this are. I doubt it's good, but I'm not sure what the best mechanism is for profiling the performance there. @cmaycumber are you familiar with any good performance measurement techniques? Now, one thing that could work: There is a parent component that is responsible for I'm mostly thinking as I type, so apologies if this is a bit all over the place. BTW, the above code would only be for server-side rendered apps. Normal RNW apps would be the same as they are now. |
I'm doing some testing of this method now and it seems to be working really well. |
Scratch that, it does have a flash of unstyled elements 🤕 |
Maybe this could work? https://github.com/artsy/fresnel |
Also relevant discussion to the artsy solution: https://artsy.github.io/blog/2019/05/24/server-rendering-responsively/ Fresnel seems promising |
Fresnel does look promising. It also looks like they have put a lot of thought into the solution as well with both support for gatsby and next. Could you see us creating a similar solution based on their approach? Or somehow integrating fresnel directly into the application? If we did implement a fresnel like solution would the match-media like API replace the theme-ui array API for most responsive use cases or would they be used side by side? |
All good questions. I'm currently experimenting with integrating Fresnel in directly. The API for I'm imagining something like this at the moment: // create-themed-component.ssr.tsx
import React, { ComponentType, ComponentProps } from 'react'
import { SSRMediaQuery } from '../provider' // this is just the Media component from fresnel, renamed
import { ThemedOptions, StyledProps } from './types'
import { useThemeUI } from '@theme-ui/core'
import { mapPropsToStyledComponent } from '.'
type Props<P> = Omit<StyledProps<P>, 'theme' | 'breakpoint'>
export function createThemedComponent<P>(
Component: ComponentType<P>,
options: ThemedOptions = {}
) {
const WrappedComponent = React.forwardRef<
typeof Component,
Props<P> & ComponentProps<typeof Component>
>(function Wrapped(prop, ref) {
const { sx, as: SuperComponent, variant, style, ...props } = prop
const { theme } = useThemeUI()
// responsiveStyles is an array of mobile-first styles for each breakpoint,
// ...generated by the css function
// ex: responsiveStyles = [{ backgroundColor: 'blue' }, { backgroundColor: 'green' }, ...]
const { responsiveStyles, ...styles } = mapPropsToStyledComponent(
{ theme, variant, sx, style },
options
)()
const TheComponent = SuperComponent || Component
if (responsiveStyles?.length) {
return (
<>
{responsiveStyles.map((breakpointStyle = {}, breakpointIndex) => {
const responsiveProps = {}
if (breakpointIndex === responsiveStyles.length - 1) {
// for the last item in the array, it should go from here until larger sizes
responsiveProps.greaterThanOrEqual = `${breakpointIndex}`
} else {
responsiveProps.at = `${breakpointIndex}`
}
return (
<SSRMediaQuery
// not sure how to get a truly unique key here yet...
key={`ssr-media-query-${
Component.displayName
}-${breakpointIndex}-${JSON.stringify(breakpointStyle)}`}
{...responsiveProps}
>
<TheComponent
{...((props as unknown) as P)}
ref={ref}
style={{ ...styles, ...breakpointStyle }}
/>
</SSRMediaQuery>
)
})}
</>
)
}
return (
<TheComponent {...((props as unknown) as P)} ref={ref} style={styles} />
)
})
WrappedComponent.displayName = `Themed.${Component.displayName ??
'NoNameComponent'}`
return React.memo(WrappedComponent)
} The idea of this approach is to still take in an In the approach above, responsive styles get added to an array of style objects for each breakpoint, called For instance: <View sx={{ bg: ['blue', 'green'] /> Would lead to: const responsiveStyles = [{ backgroundColor: 'blue' }, { backgroundColor: 'green' }, { backgroundColor: 'green }', { backgroundColor: { backgroundColor: 'green' }] The reason The downside of this is that it only allows for array breakpoints, and not custom object values like theme ui has. But 1) I don't think that really matters much, and 2) could probably fix that down the road. I have a decent start on generating the |
By the way, the above would only apply for apps that want to use Something like: // index.js
import { createThemedComponent as createNativeComponent } from './create-native-themed-component'
import { createThemedComponent as createSSRComponent } from './create-ssr-themed-component'
import { Platform } from 'react-native'
import { dripsyOptions } from '../provider'
export let createThemedComponent = createNativeComponent
if (Platform.OS === 'web' && dripsyOptions.ssr === true) {
createThemedComponent = createSSRComponent
} |
I think what you outlined is pretty elegant. If the main downside is that you don't get the custom object values that theme-ui supports I don't think that's the biggest deal either. In most cases, it seems that theme-ui tends to prefer the array-based approach anyway. I've personally have experimented with using objects with keys for responsive styling in the past and quickly moved away from it because it was almost always less clear + more code to write.
Anything stopping you from using a UUID? With both next static pages and gatsby, the computation would happen at build time. What's the biggest challenge in getting it working? The logic seems like it works. If you push any of these changes to a new branch I can take a crack I trying to get some of it to work. |
Sure thing, would be great, I can push the branch I'm working on. A lot of the code in the |
Good point. But if the component re-renders, it'll generate a new key that isn't consistent across renders, right? |
That's a valid point it would cause the key to generate which would be bad news. Maybe there's a good way of handling it. I'll give it some thought. |
I think a potential solution to the key issue would be to generate a key at build time using a UUID then checking the Component. _reactInternalFiber.key field to determine if the key exists and if it does use that key instead of generating a new UUID. You could then use this key to add to the key for the individual media query components. I think that this should cover almost every scenario because if a key was manually set, that one would be used instead and has to be unique relative to its position on in the DOM tree. Maybe there are some edge cases I'm not getting, let me know if you think of any. |
I'll look into that. Do you have an idea of what the code might look like if we use internal fiber? Some relevant content: https://reactjs.org/docs/lists-and-keys.html#keys-must-only-be-unique-among-siblings
This makes me think that worst case the breakpoint indices might be sufficient for identifying each breakpoint. If there is any responsive style for a given |
I saw that as well. The only thing that I was slightly worried about and couldn't really find anything on is whether or not the fragment would prevent items from being rendered as siblings or not. I could see a case where something might cause a collision if someone used the same style with the same responsive index on the same level as another sibling. I think it would be incredibly rare so what you have might be good enough. I imagined something like this: export function createThemedComponent<P>(
Component: ComponentType<P>,
options: ThemedOptions = {}
) {
const WrappedComponent = React.forwardRef<
typeof Component,
Props<P> & ComponentProps<typeof Component>
>(function Wrapped(prop, ref) {
const { sx, as: SuperComponent, variant, style, ...props } = prop
const { theme } = useThemeUI()
// responsiveStyles is an array of mobile-first styles for each breakpoint,
// ...generated by the css function
// ex: responsiveStyles = [{ backgroundColor: 'blue' }, { backgroundColor: 'green' }, ...]
const { responsiveStyles, ...styles } = mapPropsToStyledComponent(
{ theme, variant, sx, style },
options
)()
const key = Component._reactInternalFiber.key ?? uuid();
const TheComponent = SuperComponent || Component
if (responsiveStyles?.length) {
return (
<React.Fragment key={key}>
{responsiveStyles.map((breakpointStyle = {}, breakpointIndex) => {
const responsiveProps = {}
if (breakpointIndex === responsiveStyles.length - 1) {
// for the last item in the array, it should go from here until larger sizes
responsiveProps.greaterThanOrEqual = `${breakpointIndex}`
} else {
responsiveProps.at = `${breakpointIndex}`
}
return (
<SSRMediaQuery
// not sure how to get a truly unique key here yet...
key={`${key}-ssr-media-query-${
Component.displayName
}-${breakpointIndex}-${JSON.stringify(breakpointStyle)}`}
{...responsiveProps}
>
<TheComponent
{...((props as unknown) as P)}
ref={ref}
style={{ ...styles, ...breakpointStyle }}
/>
</SSRMediaQuery>
)
})}
</React.Fragment >
)
}
return (
<TheComponent {...((props as unknown) as P)} ref={ref} style={styles} />
)
})
WrappedComponent.displayName = `Themed.${Component.displayName ??
'NoNameComponent'}`
return React.memo(WrappedComponent)
} If it works how I imagine the key should be caught when the item rerenders and therefore would be stable. Again it might not really be necessary but it might work giving a shot. |
Ah, I see. I was referring specifically to the key for the |
Separately, here is the |
Could you elaborate on what you mean? As long as the indices are different and consistent across re-renders, each breakpoint item should be uniquely identified, right? |
PR, with code not working yet... #13 |
A concern for when this works: I wonder if this should be moved to a |
I think that this solves the issue I thought we might run into. I did a quick test to see if you could render two items with fragments wrapping them with the same key and react seems fine with it. Your original solution is the way to go because I think you're right it will always be unique relative to its position in the tree.
I don't think that's a bad idea. A monorepo might help organize things when the library starts to grow. |
I got this working on a new branch, looking really smooth. I'm going to clean it up a bit and then make a PR. It seems like it's working fully with Next.js / SSR without any flash of unstyled content. |
@nandorojo that's awesome! That would be a big win. |
@cmaycumber Any shot you'd be able to try the app on mobile for #14? I'm getting some errors running |
@nandorojo Yes, I'll test this out. I'll try running it on both the example app and another application I'm currently using with dripsy. |
@nandorojo Good news! I just tested out your fresnel-2 branch in a non-expo react-native app on both android and iOS, as well as next.js with react-native-web. I checked breakpoints across these platforms and they seem to work. However, in next.js, my custom breakpoints I set on the theme are not applying. In native, the responsive arrays respond to the correct breakpoints I have set. On next.js, they seem to still respond to the default breakpoints. |
@alexkrautmann thanks so much! That's great. Yup, you're correct about the breakpoints. This branch doesn't currently support custom breakpoints for web, since fresnel declares the breakpoints outside of render code. I will be adding support for that once I confirm that this branch works for every platform, including expo. I'll add that to the to-do of that PR. |
If you try to use this in an expo + Next.js project, you see an error stating that the prop differed on server and client. This appears to be a next.js issue.
The text was updated successfully, but these errors were encountered: