Skip to content

Conversation

rjwats
Copy link
Owner

@rjwats rjwats commented May 21, 2022

No description provided.

@rjwats
Copy link
Owner Author

rjwats commented May 21, 2022

Is anyone able to give this a test/review for me?

@proddy
Copy link

proddy commented May 21, 2022

<RouterTabs> is missing the children property?

@rjwats
Copy link
Owner Author

rjwats commented May 21, 2022

Humm.. shouldn't be, are you getting an error?

image

@rjwats
Copy link
Owner Author

rjwats commented May 21, 2022

try an "npm ci" - could be a deps issue?

@proddy
Copy link

proddy commented May 21, 2022

try an "npm ci" - could be a deps issue?

no, your repo works just fine. The problem is on my side with my project. If apply the same changes for react18 to my fork I get:

Screenshot 2022-05-21 174628

@rjwats
Copy link
Owner Author

rjwats commented May 21, 2022

Oh right, this is a change in React 18:

FC used to be:

image

and now is:

image

So children must be declared, there's a helper you can use to mix these into existing interfaces, or you can do this like i'm doing in my PR with a utility type your props interfaces can extend from :)

@rjwats rjwats merged commit 004659b into master May 21, 2022
@rjwats rjwats deleted the upgrade-e32-4.3.0 branch May 21, 2022 16:44
@proddy
Copy link

proddy commented May 24, 2022

BTW I also modifed /src/index.tsx to

import { StrictMode } from 'react';
import { createRoot } from 'react-dom/client';
import { BrowserRouter } from 'react-router-dom';

import App from './App';

const root = createRoot(document.getElementById('root') as HTMLElement);

root.render(
  <StrictMode>
    <BrowserRouter>
      <App />
    </BrowserRouter>
  </StrictMode>
);

as per https://reactjs.org/blog/2022/03/08/react-18-upgrade-guide.html#updates-to-client-rendering-apis

@rjwats
Copy link
Owner Author

rjwats commented May 24, 2022 via email

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