Skip to content

migration of @nivo/core to typescript and (partial) upgrade to React 18#2046

Draft
tkonopka wants to merge 17 commits intoplouc:masterfrom
tkonopka:core-typescript
Draft

migration of @nivo/core to typescript and (partial) upgrade to React 18#2046
tkonopka wants to merge 17 commits intoplouc:masterfrom
tkonopka:core-typescript

Conversation

@tkonopka
Copy link
Copy Markdown
Contributor

Addresses #1219, #884 (partially)

Summary

  • migrated @nivo/core to typescript
  • moved @nivo/recompose from packages to deprecated.
  • merged @nivo/tooltip into @nivo/core. They had mutual peer dependencies, and I couldn't get them to build separately.
  • disabled stories and tests for non-typescript packages (affects line and waffle, also geo and parallel-coordinates). These packages are non-functioning in this branch.
  • updated dependencies to React 18.1
  • upgraded storybook to 6.5.9 to gain React 18 support
  • adjusted all packages to use the new @nivo/core and pass build/lint/test
  • further details are described in file packages/core/migration.md

Checks

  • passes make init (building all packages from scratch)
  • passes make packages-lint
  • passes make packages-test (but non-typescript tests are disabled)
  • runs make storybook and produces working charts (but non-typescript charts are disabled)

Open issues / questions

  • compatibility with website
  • storybook reports a warning about ReactDOM.render. It seems that despite having React 18 available, the pages run as if in React 17 mode. So compatibility with React 18 is unclear. (can someone help?)

@plouc, I set this as 'draft' to signal that it shouldn't be merged at this stage. But I hope some of it is useful. With your comments, we can extend this to address the issues, or move some material into smaller updates. Thanks!

@codesandbox-ci
Copy link
Copy Markdown

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@willemmulder
Copy link
Copy Markdown

Great work!

@kacper-rogowski-wttech
Copy link
Copy Markdown

hi @plouc ,
Is there a chance, for you to take a look at this PR?

@stale stale Bot added the stale label Jan 15, 2023
@willemmulder
Copy link
Copy Markdown

Bump pleeease! :-)

@stale stale Bot removed the stale label Jan 16, 2023
@srowe0091
Copy link
Copy Markdown

srowe0091 commented Apr 21, 2023

storybook reports a warning about ReactDOM.render. It seems that despite having React 18 available, the pages run as if in React 17 mode. So compatibility with React 18 is unclear. (can someone help?)

@tkonopka React 18 introduces a new way to render to the dom. here is what that looks like

// Before
import { render } from 'react-dom';
const container = document.getElementById('app');
render(<App tab="home" />, container);

// After
import { createRoot } from 'react-dom/client';
const container = document.getElementById('app');
const root = createRoot(container); // createRoot(container!) if you use TypeScript
root.render(<App tab="home" />);

PS. would love to get this into React 18

@tkonopka
Copy link
Copy Markdown
Contributor Author

Hi @srowe0091. Yes, but that setup code is managed by storybook (nivo packages add components to an app that is already running). The appropriate initialization is supposed to be automatic, but here storybook seems to pick the pre-18 approach, which means the configuration or dependencies need adjustment. In a different project, I managed the switch by re-installing storybook from scratch, so that is an option.

Repository owner deleted a comment from stale Bot May 8, 2023
@plouc plouc added the pinned label May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants