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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Hooks internally (aka 7.0) #1209

Merged
merged 40 commits into from Apr 9, 2019

Conversation

@timdorr
Copy link
Member

timdorr commented Mar 22, 2019

馃敟馃敟馃敟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.)

markerikson and others added some commits Mar 18, 2019

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
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
Remove batch arg from createListenerCollection (#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
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.
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.
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.
@shapeshifta78

This comment has been minimized.

Copy link

shapeshifta78 commented Mar 27, 2019

Is there a roadmap for the upcoming releases anywhere? Maybe I am just blind... <3

@markerikson

This comment has been minimized.

Copy link
Contributor

markerikson commented Mar 27, 2019

@shapeshifta78 : see #1177: React-Redux Roadmap: v6, Context, Subscriptions, and Hooks

Basically, the plan is:

  • Make sure the v7 beta is completely stable and good to go
  • Release v7 final
  • Figure out what in the world we're doing regarding a public hooks API
@shapeshifta78

This comment has been minimized.

Copy link

shapeshifta78 commented Mar 27, 2019

@markerikson Thanks for the quick reply ;)

@markerikson

This comment has been minimized.

Remove WebStorm gitignore
This goes in a global gitignore file, not a project.
@timdorr

This comment has been minimized.

Copy link
Member Author

timdorr commented Apr 3, 2019

That's an enyzme-to-json bug: adriantoine/enzyme-to-json#137

@nperichSYKES

This comment has been minimized.

Copy link

nperichSYKES commented Apr 3, 2019

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.

josepot and others added some commits Apr 4, 2019

@markerikson

This comment has been minimized.

Copy link
Contributor

markerikson commented Apr 4, 2019

Just published 7.0.0-beta.1, with a fix from #1220 that should resolve the "one update behind" issue reported in #1219 .

@kbrandwijk

This comment has been minimized.

Copy link

kbrandwijk commented Apr 4, 2019

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...

@timdorr

This comment has been minimized.

Copy link
Member Author

timdorr commented Apr 4, 2019

One last request before release: Can we move to react-hooks-testing-library? https://www.npmjs.com/package/react-hooks-testing-library

@markerikson

This comment has been minimized.

Copy link
Contributor

markerikson commented Apr 4, 2019

@timdorr : given that we're not using any actual custom hooks atm, is that lib actually necessary in any way?

@markerikson

This comment has been minimized.

Copy link
Contributor

markerikson commented Apr 4, 2019

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.

@markerikson

This comment has been minimized.

Copy link
Contributor

markerikson commented Apr 4, 2019

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!

@timdorr

This comment has been minimized.

Copy link
Member Author

timdorr commented Apr 4, 2019

Oh, my bad. They just extracted testHook out, which we aren't using. Nevermind!

@kelseasy

This comment has been minimized.

Copy link

kelseasy commented Apr 4, 2019

Just giving some external feedback:

  • Going from v6 to v7 fixed some issues with mapStateToProps being called again with a stale state
  • There is a small breakage for people using rollup for libraries that don't bundle react-dom, as the dependency on react-dom is now explicit due to unstable_batchUpdates. The fix is to have that dependency explicit in rollup.config.js:
{
  plugins: [
    commonjs({
      include: "node_modules/**",
      namedExports: {
          "node_modules/react-dom/index.js": ["unstable_batchedUpdates"]
        }
      })
  ]
}
  • Definitely faster too!

Thanks for the hard work around that release! 馃挅

@markerikson

This comment has been minimized.

Copy link
Contributor

markerikson commented Apr 4, 2019

@kelseasy : can you clarify what the build issue is, exactly? And is that suggested tweak for our Rollup config, or someone else's?

@kelseasy

This comment has been minimized.

Copy link

kelseasy commented Apr 4, 2019

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 react-dom) will have to make that change. Sorry that wasn't clear!

@asundlihardig

This comment has been minimized.

Copy link

asundlihardig commented Apr 8, 2019

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 馃檶馃徎

@markerikson

This comment has been minimized.

Copy link
Contributor

markerikson commented Apr 9, 2019

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 major.0 release.

Let's do this.

@markerikson markerikson merged commit fa58572 into master Apr 9, 2019

4 checks passed

codecov/project 97.74% (-0.49%) compared to 28adb54
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
netlify/react-redux-docs/deploy-preview Deploy preview ready!
Details

@reduxjs reduxjs deleted a comment from abumalick Apr 9, 2019

@timdorr timdorr deleted the v7-beta branch Apr 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.