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 ideas for usage ergnomics in absence of anonymous components #17

Closed
lewisl9029 opened this issue Oct 28, 2021 · 3 comments
Closed

Comments

@lewisl9029
Copy link
Owner

lewisl9029 commented Oct 28, 2021

Unfortunately RFC was rejected: reactjs/rfcs#197

This means usage of the hook API will continue to be rather painful when branching/mapping is involved.

Potential pivot: a component API!

// Naming tbd
<UseStyles with={<div {...anyDivProps} />} display="flex">
  children here
</UseStyles>

Internally, implemented as:

const UseStyles = ({ with, children, ...styles }) => {
  return React.cloneElement(with, {
    className: useStyles(styles, Object.entries(styles).flat()),
    null,
    children
  })
}

Nice thing here is dependency analysis becomes automatic, unfortunately doesn't look very performant... Still worth benchmarking though.

Alternatively, a more direct translation:

// Naming tbd
<UseStyles with={div} styles={{ display: "flex" }} dependencies={[]} {...anyDivProps} >
  children here
</UseStyles>
const UseStyles = ({ with, children, styles, dependencies, ...props }) => {
  return React.createElement(with, {
    className: useStyles(styles, dependencies),
    props,
    children,
  })
}

Lastly, it might be worth trying generating the dependency list automatically from styles using the flattened entries approach above, directly inside the hook implementation (breaking change), and adopt if perf doesn't suffer significantly. Not having to supply a dependency array would improve ergonomics by a boatload. Alternatively, we can make this behavior default but allow users to optimize by supplying an optional dependency list.

https://github.com/lewisl9029/use-styles/blob/master/src/useStyles.js#L172
to

const cacheEntries = React.useMemo(() => toCacheEntries(styles), Object.entries(styles).flat())

One more idea: Let users specify if they're passing in memoized styles to begin with.

const cacheEntries = React.useMemo(() => toCacheEntries(styles), props.memoized ? styles : Object.entries(styles).flat())

Can also split into dynamic vs static styles props.

@lewisl9029
Copy link
Owner Author

As of this stackoverflow post, array.flat hasn't been optimized yet: https://stackoverflow.com/a/61416753

Might have changed, but worth comparing against the [].concat(...array) approach

@lewisl9029
Copy link
Owner Author

Now that we've verified useMemo'ing all the things actually turns out to be slower in practice, what's preventing us from offering an API like this:

const styles = useStyles()

return <div className={styles({ /* styles object here */})}></div>

It would enable calling in arbitrary locations in the tree.

We could use the styles call to add to cache (compartmentalizing it to distinguish between injected vs not), and then useLayoutEffect (eventually useInsertionEffect) in the useStyles to inject to stylesheet.

@lewisl9029
Copy link
Owner Author

New API has been implemented

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

1 participant