-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Use Hooks internally (aka 7.0) #1209
Conversation
Matches changes from v7.0.0-alpha.1
Added rtl.act() calls around dispatches and other component updates Added clarification on expected mapState calls in some places Disabled some no-longer-relevant tests per implementation Made tests run against React 16.8 by default
Matches state as of v7.0.0-alpha.2
Matches state as of v7.0.0-alpha.3
Matches state as of v7.0.0-alpha.4
This prevents a bug with Terser (webpack's default minifier) where the returned batch function isn't defined due to function inlining. Matches state as of v7.0.0-alpha.5
We were trying to read contextValue.subscription, even if that value was null. Reworked logic to handle cases where the store came in as a prop.
Yeah, this kills the blame history. Sorry. But it's a lot easier to figure out what the tests are used for now.
Changed to useLayoutEffect to ensure that the lastWrapperProps ref is written to synchronously when a render is committed. However, because React warns about this in SSR, added a check to fall back to plain useEffect under Node so we don't print warnings. Also added logic to ensure that if an error is thrown during a mapState function, but the component is unmounted before it can render, that the error will still be thrown. This shouldn't happen given our top-down subscriptions, but this will help surface the problem in our tests if we ever break the top-down behavior.
@shapeshifta78 : see #1177: React-Redux Roadmap: v6, Context, Subscriptions, and Hooks Basically, the plan is:
|
@markerikson Thanks for the quick reply ;) |
Bug report regarding HOC display names: https://twitter.com/Cl0udW4lk3r/status/1113171785929949184 |
This goes in a global gitignore file, not a project.
That's an enyzme-to-json bug: adriantoine/enzyme-to-json#137 |
I didn't upgrade to 6.0 because I hit some edge case bug that I never managed to figure out (didn't spend much time on it as 5.0 already worked fine). But based on a quick spot check, 7.0 seems to work perfect! I definitely don't get the same problem, whatever it was. Anyway, 7.0 is working good for me so far on a reasonably complex app. |
Using 7.0 in production since the beta release. Just wanted to let you know that I haven't experienced any issues so far! It also 'feels' faster, but I don't have any metrics to support that feeling... |
One last request before release: Can we move to react-hooks-testing-library? https://www.npmjs.com/package/react-hooks-testing-library |
@timdorr : given that we're not using any actual custom hooks atm, is that lib actually necessary in any way? |
One other thought to throw out there. Given what we've been seeing with v6, it might be worth updating the "Cannot find store" error message to suggest examining build config or something. |
After discussion with Tim, I'm tentatively planning to publish v7 as final early next week if no new issues pop up. PLEASE KEEP TESTING THE BETA, FOLKS! |
Oh, my bad. They just extracted testHook out, which we aren't using. Nevermind! |
Just giving some external feedback:
{
plugins: [
commonjs({
include: "node_modules/**",
namedExports: {
"node_modules/react-dom/index.js": ["unstable_batchedUpdates"]
}
})
]
}
Thanks for the hard work around that release! 💖 |
@kelseasy : can you clarify what the build issue is, exactly? And is that suggested tweak for our Rollup config, or someone else's? |
Oh sorry, it was my rollup config that needed more info, that change is already in your rollup config. Which means that every downstream consumer (that isn't bundling |
I installed 7.0.0-beta.1 on two projects in production. They have been running for a couple of days now. No issues so far 🙌🏻 |
Well, if there's one thing my experience as a maintainer has taught me... it's that the only way to guarantee getting feedback is to publish a new Let's do this. |
* Update React to latest * Update React peer dependency to 16.8.x * Initial re-implementation of `connectAdvanced` using hooks Matches changes from v7.0.0-alpha.1 * Update tests to match v7-alpha.1 behavior Added rtl.act() calls around dispatches and other component updates Added clarification on expected mapState calls in some places Disabled some no-longer-relevant tests per implementation Made tests run against React 16.8 by default * adding a react hooks test that fails with v7 alpha * wrapping store.dispatch with rlt.act, fixes component renders * reducing hooks test to 2 components * Fix case where wrapper props changed before store update render * Mark ReactDOM as global in UMD builds Matches state as of v7.0.0-alpha.2 * Fix perf problems with out-of-bounds array access Matches state as of v7.0.0-alpha.3 * Add modules to handle importing batchedUpdates * Use appropriate batched update API for subscriptions * Inject unstable_batchedUpdates in default entry point * Provide an alternate entry point for alternate renderers Matches state as of v7.0.0-alpha.4 * Remove batch arg from createListenerCollection (reduxjs#1205) This prevents a bug with Terser (webpack's default minifier) where the returned batch function isn't defined due to function inlining. Matches state as of v7.0.0-alpha.5 * Remove older React versions from Travis * Add comments to connectAdvanced. Many much comments! * Re-add test for a custom store as a prop * Fix null pointer exception when store is given as a prop We were trying to read contextValue.subscription, even if that value was null. Reworked logic to handle cases where the store came in as a prop. * Ensure wrapper props are passed correctly when forwarding refs * Add a test to verify subscription passthrough with store-as-prop * add non-batched tests (reduxjs#1208) * Force SSR tests to mimic a Node environment * Restructure connect tests to group by category for easier reading Yeah, this kills the blame history. Sorry. But it's a lot easier to figure out what the tests are used for now. * Clean up dead code in Provider tests * Add tests to verify errors are thrown for bad mapState functions * Fix edge cases around saved wrapper props and error handling Changed to useLayoutEffect to ensure that the lastWrapperProps ref is written to synchronously when a render is committed. However, because React warns about this in SSR, added a check to fall back to plain useEffect under Node so we don't print warnings. Also added logic to ensure that if an error is thrown during a mapState function, but the component is unmounted before it can render, that the error will still be thrown. This shouldn't happen given our top-down subscriptions, but this will help surface the problem in our tests if we ever break the top-down behavior. * Formatting * Add a test to verify no errors are printed in SSR usage * Ignore .idea/ * 7.0.0-beta.0 * Updated outdated SSR-test (dispatch in ancestors) (reduxjs#1213) * Added test for injecting dynamic reducers on client and server (reduxjs#1211) * Remove WebStorm gitignore This goes in a global gitignore file, not a project. * [FIX]: reduxjs#1219 Save references before update (reduxjs#1220) * Re-ignore .idea/ * 7.0.0-beta.1 * Update the codecov config to be more forgiving. * add test to verify that mapStateToProps is always called with latest store state (reduxjs#1215)
🔥🔥🔥React-Redux v7 beta is out! 🔥🔥🔥 We've just published our first beta of React-Redux version 7, and it should be the fastest version of React-Redux yet! Please try it out in your production apps, and let us know how well it works for you!
https://github.com/reduxjs/react-redux/releases/tag/v7.0.0-beta.0
We've rewritten connect to use hooks internally, switched back to using direct store subscriptions in components, and we're now using React's "batched updates" API to reduce re-renders. v7 is API-compatible with v6 - the major bump is primarily because we require React 16.8.
We've also added a new batch() API you can import and call in your own code to minimize re-renders with multiple dispatches. connect and batch should work correctly in ReactDOM and React Native, and there's a new alternate entry point to use with alternate React renderers.
We've tested and benchmarked, and are confident this beta is ready for you to try out in your own apps. We've got a lot of users who do things we don't know about, so if you do find an issue, please let us know in this issue.
(the other issue is getting all Big McLargeHuge.)