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

Upgrade to React v17 and R3F v6 #603

Merged
merged 3 commits into from
Apr 19, 2021
Merged

Upgrade to React v17 and R3F v6 #603

merged 3 commits into from
Apr 19, 2021

Conversation

axelboc
Copy link
Contributor

@axelboc axelboc commented Apr 13, 2021

Time for the React v17 and R3F v6 upgrade! There are still a bunch of peer-deps warnings, but much fewer than before.

No changes from React 17, of course, but R3F had a bunch of breaking changes, not all of which were documented...

Notable improvements:

  • No need for attach="material" and attach="geometry".
  • useThree is now based on zustand, so we can pass it a selector to avoid unnecessary re-renders. When we need to get multiple properties from the state, I chose to write separate useThree(state => state.prop) calls instead of using zustand's shallow comparator. Having separate calls allows for destructing the objects we get, like size, camera, etc. right away.
  • WebGL 2 is now the default.

Other notable upgrades:

  • Three is still making progress with reducing its tree-shaken bundle size.
  • d3-array has a fix to d3.ticks that may benefit us somewhere.
  • The ESLint config now warns us when we call a function with arguments of type any. I've had to fix a few of those.
  • Storybook's controls add-on handles JSON arguments much better:

image


Planned work:

  • useUpdate has been deprecated, so the code in ErrorBars is not as readable. I think there's a way to make this component more declarative by passing an array of floats directly to <bufferGeometry args={[positions]} /> instead of calling setFromPoints() imperatively. Same in DataCurve.
  • I've just thought of a way to extract the code of the tooltip formatters from LineVis and HeatmapVis with a hook called useTooltipFormatters. I might give that a try.

@axelboc axelboc requested a review from loichuder April 13, 2021 13:59
Base automatically changed from splitter to main April 13, 2021 14:36
@axelboc axelboc changed the title Upgrade dependencies Upgrade to React v17 and R3F v6 Apr 16, 2021
package.json Show resolved Hide resolved
src/h5web/App.tsx Show resolved Hide resolved
src/h5web/providers/context.ts Show resolved Hide resolved
src/h5web/providers/context.ts Show resolved Hide resolved
src/h5web/providers/utils.ts Show resolved Hide resolved
src/h5web/providers/utils.ts Show resolved Hide resolved
src/h5web/vis-packs/core/line/DataCurve.tsx Show resolved Hide resolved
src/h5web/vis-packs/core/line/ErrorBars.tsx Show resolved Hide resolved
src/h5web/vis-packs/core/shared/Html.tsx Show resolved Hide resolved
Copy link
Member

@loichuder loichuder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not much to say: well done

@axelboc axelboc merged commit bb5719d into main Apr 19, 2021
@axelboc axelboc deleted the deps branch April 19, 2021 07:14
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

Successfully merging this pull request may close these issues.

2 participants