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

Consider exporting `batchedUpdates` from React #15080

Open
markerikson opened this issue Mar 10, 2019 · 4 comments
Open

Consider exporting `batchedUpdates` from React #15080

markerikson opened this issue Mar 10, 2019 · 4 comments

Comments

@markerikson
Copy link

@markerikson markerikson commented Mar 10, 2019

ReactDOM and React Native both currently export an unstable_batchedUpdates API. Because batching is a renderer/reconciliation-level concern, this API is exported by the renderer packages, not the core react package.

The React team has recently encouraged the Redux team to make use of unstable_batchedUpdates in React-Redux. However, this becomes complicated due to how that API is being exported.

It's possible to deal with this at the bundler level. Some experimentation shows that creating an alternate file with a .native.js extension will cause that to be picked up by the RN bundler, as in this example:

// batch.js
import {unstable_batchedUpdates} from "./react-dom";

// ./react-dom.js
export {unstable_batchedUpdates} from "react-dom"

// ./react-dom.native.js
export {unstable_batchedUpdates} from "react-native"

However, this does not handle the case where an alternative React renderer is being used. The list of other React renderers is continuing to grow, which means that a React library that needs batching would have to deal with that situation in some way. This becomes extremely complicated when you start considering variations on bundlers, module formats, and build environments.

It would be extremely beneficial if the React core itself exported a batchedUpdates API. That could default to being a noop wrapper like (callback) => callback() if no suitable implementation was available.

I know that unstable_batchedUpdates() is, uh... "unstable". However, the React team has stated that "it's the most stable of the unstable APIs", and "half of Facebook depends on this".

I think it would really help the ecosystem if some form of this API was solidified and exported from the core React package itself.

@markerikson

This comment has been minimized.

Copy link
Author

@markerikson markerikson commented Mar 11, 2019

I suppose an alternative solution might be for userland packages to provide an alternate entry point for use with non-mainline renderers that would pull in a fallback noop, like:

import {connect, Provider, batch} from "react-redux/alternate-renderers";

Would still be nice to not have to worry about this, though.

@FredyC

This comment has been minimized.

Copy link

@FredyC FredyC commented Aug 24, 2019

This is something we have been dealing with mobx-react for quite some time. The current solution resides in bundling for react-dom and then making a copy of that bundle and do hard replace of react-dom to react-native.

It's a viable solution, but far from pretty as it depends heavily on the building process and cannot be easily shared. Among the recent issues, this is problematic with Jest which is unable to recognize the RN bundle by default and fails. People have to manually configure Jest to map to RN build.

I haven't inspected that closely what are differences between those two implementations and how much it depends on other platform-specific code. If it's way too different, I don't see how it could be moved under the react which would need to have some logic to recognize for which environment it's supposed to be used. Besides, it doesn't sound right to grow the package size for something that is used rarely.

I was toying with the idea of dynamic import() which can be done conditionally, but it's still a problem because Webpack (not sure about others) would still try to create chunks for both and fail for the missing one. Wrapping require to try/catch will still yield a warning and confuse the end user.

So I think that ultimately we need to devise some react-batches-bridge module that can take care of this burden. If both implementations can be extracted into that package and possibly consolidate common parts, it would solve all the pains.

@eps1lon

This comment has been minimized.

Copy link
Contributor

@eps1lon eps1lon commented Aug 24, 2019

Have you experimented with making the batchedUpdates implementation configurable? Instead of requiring the implementation let the consumer inject it. While clunky for users of popular renderers such as react-dom or react-native it would allow react-redux to be used with any renderer. You could then always include separate pre-configured bundles for react-dom or react-native using optional peer dependencies.

Or something like import { batchedUpdates } from 'react-redux/batchedUpdatesImplementation' and advise consumers to alias react-redux/batchedUpdatesImplementation' to whatever renderer they use. Just throwing out some random thoughts. Just disregard if you already tried this.

@FredyC

This comment has been minimized.

Copy link

@FredyC FredyC commented Aug 24, 2019

@eps1lon Ah, you might be onto something. Considering it's an optimization and unstable on top of that, it might be justifiable to let users opt-in. Thank you, I think we will try to go that way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.