Skip to content

Conversation

@br1anchen
Copy link
Contributor

What kind of change does this PR introduce?

@Saeris @christianalfoni
This PR adds @types/react-router-dom to app repo in order to get correct react-router-dom components typing.
Which is necessary to convert current react-router-dom components usage in js files to ts files, such as: app/src/app/pages/Dashboard/Content/routes/PathedSandboxes/Navigation/elements.js

What is the current behavior?

Currently there is no typing for all react-router-dom components, so there is no type checking for props assigned to them.
Also withRouter() hoc, can take any components no matter if it expects RouteComponentProps or not.

What is the new behavior?

This PR will introduce react-router-dom type checking for its usage.
And withRouter() hoc would expect component explicitly expects RouteComponentProps.
PS: I am not aware of why some of the component is wrapped in withRouter() but without using any RouteComponentProps, might can be simply removed usage of withRouter(), instead of current fix by introducing new props to them.

What steps did you take to test this? This is required before we can merge, make sure to test the flow you've updated.

  1. yarn typecheck

Checklist

  • Documentation
  • Testing
  • Ready to be merged
  • Added myself to contributors table

@lbogdan
Copy link
Contributor

lbogdan commented Oct 19, 2019

Build for latest commit 8061831 is at https://pr2869.build.csb.dev/s/new.

@SaraVieira
Copy link
Contributor

Thank you so much!!

@SaraVieira SaraVieira merged commit 2b10811 into codesandbox:master Oct 19, 2019
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.

4 participants