sx prop re-renders about dripsy HOT 30 CLOSED

I hadn't thought about it too much. I'm open to it, the only problem is that the pragma uses just CSS styles, whereas this lib requires hooks to track the screen width and such. I can see how it's really convenient. I personally prefer just importing components and using them, but if people find it really convenient maybe it'd be better to try to include it. We would have to wrap the entire component that is receiving the sx prop with the createThemedComponent function, rather than just inject a style function the way theme-ui does.

I don't mind importing the component either. The main use case for the jsx pragma in my experience is the ability to style components from outside libraries without needing to create a new component. Obviously this is easily alleviated by using the styled function or createThemedComponent function to wrap a third party component but it can be handy to just style it using sx without anything but the jsx pragma. Personally I'm fine with either approach and don't have a huge opinion either way.

I thought more about the performance here. I think the easiest solution is to just use a styled function:

Would this replace the sx prop entirely? I personally think the styled function is a less elegant solution than the sx prop because in my experience it often leads to writing much more code than you would with just the sx prop. It's also annoying to have to pass additional props to the styled component then redeclare the prop types for your newly styled component when using typescript.

Performance is definitely important especially when we start to get on the native side of things. I'm curious if there's a way to get the sx prop without a drop in performance or unnecessary rerenders. Does this library https://github.com/samjbmason/theme-ui-native run into a similar issue? I'm not sure if they have something that we're missing. I'd be glad to help look into a solution for this because I do feel strongly about the sx prop.

from dripsy.

I definitely view the sx prop as staying forever and being the primary API. The styled function would just be an option if you want to avoid re-renders / style your components outside of the scope. It would be strictly optional and secondary to the sx prop.

from dripsy.

@nandorojo @cmaycumber Do you think a babel plugin like this might be worth?

function App() {
  return (
    <>
      <View sx={{ backgroundColor: "black" }} />
      <View sx={{ marginTop: "100", paddingVertical: 20 }} />
    </>
  );
}

Converts to

import { useMemo as _useMemo } from "react";

function App() {
  const _sx = _useMemo(
    () => ({
      1: {
        backgroundColor: "black",
      },
      2: {
        marginTop: "100",
        paddingVertical: 20,
      },
    }),
    []
  );

  return (
    <>
      <View sx={_sx["1"]} />
      <View sx={_sx["2"]} />
    </>
  );
}

Cases

1. sx remains untouched if it contains any variables.

function App() {
  const padding = 10;
  return (
    <>
      <View sx={{ backgroundColor: "black", padding }} />
      <View sx={{ marginTop: "100", paddingVertical: 20 }} />
    </>
  );
}

will be converted to

import { useMemo as _useMemo } from "react";

function App() {
  const _sx = _useMemo(
    () => ({
      2: {
        marginTop: "100",
        paddingVertical: 20,
      },
    }),
    []
  );

  const padding = 10;
  return (
    <>
      <View
        sx={{
          backgroundColor: "black",
          padding,
        }}
      />
      <View sx={_sx["2"]} />
    </>
  );
}

This case is considered because when it contains a variable, it gets harder to find location to put useMemo.
e.g.

function App() {
    if (boolean) return null;

    const a = 10;
    return <View  sx={{padding: a}} />
}

Here, we might have to move the variable a above the condition and add useMemo.

2. Nit pick but it's hard to tell if a function returning JSX will be used as a react component.

e.g.

const Component = () => <View sx={{bg: "black" }} />

can be used as <Component /> or Component();

This makes it difficult to recognize whether useMemo should be added or not, one heuristic I can think of is if a function name starts with a capital letter it can be considered as a react component and try to memoize sx props.

from dripsy.

Dripsy v3 will automatically memoize the sx prop under the hood using stableHash from swr. See here

from dripsy.

I thought more about the performance here. I think the easiest solution is to just use a styled function:

import { styled, View } from 'dripsy'

const Background = styled(View)({
  backgroundColor: ['secondary', 'primary']
})

// or
const Background = styled(View)(props => ({
  backgroundColor: ['secondary', props.selected ? 'primary' : 'grey']
}))

And similar to styled-components, you'd declare these outside of your component code.

from dripsy.

This article (https://sebastienlorber.com/records-and-tuples-for-react) by @slorber about upcoming Records/Tuples appears to be promising. I'm not too familiar with how they'll work yet, but this is my impression.

It seems like records would help for any SX prop that only uses primitive values within objects.

As long as the nested primitive values are equal, the reference will be the same, and the component won't re-render.

{ hi: true } === { hi: true } // false
#{ hi: true } === #{ hi: true } // true

For example:

// works
const Memoized = () => <Box
  sx={#{ bg: #['blue', 'red'] }}
/>

// fails
const Fails = () => <Box
  sx={#{ bg: theme => [theme.primary, theme.secondary] }}
/>

For the case of wanting to use dynamic theme values, we could allow the sx prop to be a function, as discussed above, where it returns an immutable record:

const Styled = () => <Box
  sx={({ theme }) => {
    return #{ bg: #[theme.primary, theme.secondary] }
  }}
/>

For the function prop example here, we could have a React.memo function that compares prevProps.sx({ theme }) === nextProps.sx({ theme }), and not re-render if they match.

This wouldn't work for things like Animated values, but assuming I understand this proposal correctly and it gets approved, 95% of cases where memoization is needed would be solved.

from dripsy.

This project from @natew looks very interesting. He mentioned to me on Twitter that he plans to use the array syntax for media queries, too. Since snack UI figured out a way to compile down to HTML views, using it under the hood could mean abandoning Fresnel for an approach that is closer to the metal. This assumes Snack UI ends up figuring out how to do media queries with CSS, meaning it's compatible with SSR.

(It also assumes all styles map from react native primitives to web styles the way that RNW intends. I haven't tested, but it would appear that it does.)

Link: https://github.com/natew/snackui

The readme mentions the issue of missing className support in RNW, which is what led Dripsy to use the Fresnel hack in the first place.

from dripsy.

Good question...I'm actually not sure. Judging by this plugin alone, I think the answer is no, since it seems like they use JS client-side media queries and thus don't support SSR.

If we built on top of it, possibly.

It seems like a key benefit we'd get is that any Views that don't use RN logic would be compiled down to simple divs, and their styles would use optimized CSS rather than recomputing constantly. Apparently this leads to much snappier rendering.

As for getting rid of fresnel, this would require applying our styles in pure CSS. Maybe we could add this feature for the simple divs, but any RN View with more logic would probably still require fresnel. The only workaround would be to patch-package RNW to allow className, which doesn't seem like a great idea.

On another note, I have found the Sizer component I mentioned in another issue super useful in my app. I also creates a native div and uses theme-ui for breakpoints, meaning it only uses CSS. It's nice for flex boxes and such. I plan on adding it to Dripsy's core.

from dripsy.

Not sure if this is 100% related but it does relate to the sx prop. Is jsx pragma something that we want to consider including?

Now that you brought this up I'm not sure what the performance implications might be or if it's even necessary if we export the majority of components, but I can see a scenario where someone might want to style a component outside of the library with the sx prop.

from dripsy.

Is jsx pragma something that we want to consider including?

I hadn't thought about it too much. I'm open to it, the only problem is that the pragma uses just CSS styles, whereas this lib requires hooks to track the screen width and such. I can see how it's really convenient. I personally prefer just importing components and using them, but if people find it really convenient maybe it'd be better to try to include it. We would have to wrap the entire component that is receiving the sx prop with the createThemedComponent function, rather than just inject a style function the way theme-ui does.

from dripsy.

I'd love to look into performance optimizations for the sx prop too, since I'll rely on this API often as well. I'll likely focus more on getting the SSR side to work if you want to look into the perf side of this prop

from dripsy.

This may be relevant https://github.com/FormidableLabs/react-fast-compare

from dripsy.

Another idea thinking about this. Not the most "simple", but dripsy could implement something like https://github.com/andywer/use-inline-memo and provide a useSx hook that lets you generate memoized styles.

from dripsy.

I'll close this once the styled issues are fixed, and when the style prop is moved to StyleSheet.create()

from dripsy.

What does it take to move the style prop over to Stylesheet.create(). Is it a matter of simply wrapping the styles as they are being processed in the css function?

from dripsy.

Yeah, I believe so. I think I'd turn them into stylesheet objects in useMemo in createThemedComponent. It's unclear to me how much better this would be, but I see on RNW's docs that it's recommended for better performance. As for the responsiveSSRStyles, I'd probably have to loop through each one of these and use StyleSheet.create for each.

I think it would have no effect on native, where StyleSheet.create is nothing more than a JS object.

from dripsy.

I'm bubbling this issue back up.

Lately I've been thinking about static extraction of styles.

This tweet prompted me to think about performance solutions.

Ideally, any sx prop that receives an object inline that is static (ie doesn't use any variables from render code), should get turned into an object outside of render code. This means that any static styles get generated at build time, not runtime. This prevents react from recalculating styles on the fly. We could also use StyleSheet.create outside of render code.

Perhaps this could be done as some sort of Babel/Webpack plugin. I wish I had experience with either, but I don't. It could fit nicely with the JSX pragma, perhaps(?)

Some inspiration for the idea:

• https://github.com/martinandert/babel-plugin-css-in-js
• https://github.com/z0al/react-native-styled.macro
• https://github.com/jsxstyle/jsxstyle/ (see webpack plugin) this seems the most promising. At the very least, static extraction could be useful for web. I haven't noted as much of a need for it on native.
• https://babeljs.io/docs/en/babel-plugin-transform-react-constant-elements/

from dripsy.

Could use this maybe: https://github.com/kentcdodds/babel-plugin-macros

Also interesting but don't think it applies: https://github.com/kentcdodds/babel-plugin-preval

from dripsy.

from dripsy.

Got it, good to know, thanks @slorber. I'll look into that.

I got some guidance from it here. It seems that it might be a bit complicated, given that we have to traverse the root app theme to determine if styles are constants. Not really sure since I'm so new to Babel.

from dripsy.

More discussion on this at necolas/react-native-web#1801 and react-native-community/discussions-and-proposals#254 (comment).

from dripsy.

I've been looking more at Snack UI: https://github.com/snackui/snackui

The static and Babel plugin truly go over my head, but it might be worth trying to learn how this all works. Seems like it makes pretty big performance strides.

from dripsy.

Yeah, it looks pretty cool. From my understanding, if we were to create a babel plugin, would we be able to drop fresnel in lieu of media queries?

from dripsy.

This is certainly a lower priority, but I might look into it as a weekend project.

from dripsy.

Recent optimizations with stylesheet should solve most of this, since it's now cached and stringified.

from dripsy.

@intergalacticspacehighway very cool! what do you think of this case?

const Component = () => {
  const renderBox = () => <View sx={{ bg: 'background' }} />

  return <View>{renderBox()}</View>
}

I also was thinking I could use the new SWR hashing function to deep compare sx props under the hood, created from this issue: vercel/swr#1429

from dripsy.

@nandorojo This output was generated from the plugin. The plugin is simple, it works like this.

1. Verify function starts with capital letter (To detect react component)
2. Traverse all the JSX elements and search for sx attribute that is an object. e.g. sx={{x: y}}
3. Check if sx doesn't have a variable, If not, prepare to put it in useMemo or else don't touch it.
4. Construct the memo object and put it at the start of the function and replace sx attributes.

I can push a version and a playground file where one can test all the cases.

import { useMemo as _useMemo } from "react";

const Component = () => {
  const _sx = _useMemo(() => ({
    1: {
      bg: 'background'
    }
  }), []);

  const renderBox = () => <View sx={_sx["1"]} />;

  return <View>{renderBox()}</View>;
};

The hashing function looks cool! Maybe we can test the performance with new hash function on any app and try the babel plugin as well and verify if it adds any overall benefit.

from dripsy.

Technically, if there are no variables, the sx variable could be outside of render. However, this could make things more complicated if there are multiple components, etc

from dripsy.

Yes. This can be done by creating global like object and using it for each component with an identifier. I still think useMemo "might" be better as it'll be shortlived and gets garbage collected on component unmount. This needs performance testing to confirm though.

from dripsy.

Yeah the garbage collection is a good point

from dripsy.