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

React 18 Type changes #1027

Draft
wants to merge 22 commits into
base: develop
Choose a base branch
from
Draft

React 18 Type changes #1027

wants to merge 22 commits into from

Conversation

tordans
Copy link
Contributor

@tordans tordans commented Apr 11, 2022

With React 18, the Types for React.FC changed from including children props to not including them.

Now, React.FC and React.VFC are the same, see https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/react/index.d.ts#L508-L526

Changes

  • Add children props explicitly where we use React.FC (not all React.FC did in fact use the children props, so some stay as they are)
  • Remove React.VFC in favor of React.FC

TODOs

  • We still need to run type check on this change once we update React.
  • Rebase on master once we are ready

FYI, es gibt noch mehr deprecated types https://github.com/eps1lon/types-react-codemod#all-deprecated--transforms

- extract menu, routes as names export
- rename default route to `.landing`.
- Add first set of content with dummy maps and images for CycleNetworkDevelopmentPage
- Rename ZesPlusResearchPage to follow the *Page pattern
- use named exports
- move maps from components to page-content-folder
- add types in a more standard way
- Extract flying legend to component
- Standardize import/export of planning legend
The linked extension was not available anymore. This one looks more official.
- Message as children
- Make TS
- Move to components2
- Move icons to component folders
- Cleanup SVGs by removing filter and duplicate circle
- Refactor LocatorControl to TS into components2
- Extract LocatorButton from LocatorControl and use that inside the Planung/HBI map, so all Button have the same behavior
- Refactor Styling of MapButtons so they all look the same; and less duplicated styles. Also some hover styles and such.
- Refactor the layer show/hide feature to be used in the fullscreen map; remove it from the previous test page since the interface is now different
- Add a test map on the ZES+ page to test the fullscreen map
- This includes a debug panel that allows to show/hide all layers of the current style. HOWEVER, there is something out of sync here (likely due to async data processing internally in mapbox) so the visibility state is only correct after the next click).
This whole folder was only used by the Map-"Product" (HBI/Planungen) (by now), so we should move it in the component folder over there.

Also import styles directly from the styles file and rename that so it's easier to understand the imports.
Using `css` here activates the code styling; and does not change the outcome.
- Rename ModalCloseIcon component to ClosePanelButton component
- Remove duplicated CloseMapIcon component in favor of reusing ClosePanelButton
- Refactor components to use `style` props which overwrite the given styled-components styles automatically. This means, we need to write less code in styled components.
- Allow MapsControl to be hidden via props
- Add styles for FullscreenMap on mobile
- Cleanup FullscreenMap comments and props
This was an experiment that we do not need anymore. The rest of the code was already removed.
Its used by the Article/Map and Article/FullscreenMap.
Lets use true/false props here for position which makes the code a lot easier.

Also fixes the visibility of the button on desktop.
- Use named export
- Consolidate the types in a /config/types folder
- Simplify the types where possible; did not manage to get rid of the per route-types, though; but they are simplified
- Add types where missing
Remove React.VFC.

This is in preparation for React 18.
@tordans tordans marked this pull request as draft April 11, 2022 07:16
@tordans tordans changed the base branch from develop to zesplus-update-2022-03 April 11, 2022 07:16
@codecov
Copy link

codecov bot commented Apr 11, 2022

Codecov Report

Merging #1027 (891c216) into zesplus-update-2022-03 (d32ea42) will not change coverage.
The diff coverage is 95.45%.

@@                   Coverage Diff                   @@
##           zesplus-update-2022-03    #1027   +/-   ##
=======================================================
  Coverage                   36.55%   36.55%           
=======================================================
  Files                         524      524           
  Lines                        9594     9594           
  Branches                     1974     1969    -5     
=======================================================
  Hits                         3507     3507           
  Misses                       5807     5807           
  Partials                      280      280           
Flag Coverage Δ
unittests 36.55% <95.45%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/apps/Gastro/components/FileUpload.tsx 75.00% <ø> (ø)
src/apps/Map/components/MapLegend/Legend.tsx 0.00% <0.00%> (ø)
...omponents2/Article/FullscreenMap/FullscreenMap.tsx 34.21% <ø> (ø)
src/components2/Article/Image/ImageMulti.tsx 100.00% <ø> (ø)
...rc/components2/Article/Map/MapActivationButton.tsx 84.61% <ø> (ø)
src/components2/Article/Quote/FaceQuote.tsx 93.75% <ø> (ø)
src/components2/Article/Quote/Quote.tsx 100.00% <ø> (ø)
src/components2/Article/Typography/Details.tsx 83.33% <ø> (ø)
src/components2/ErrorMessage/ErrorMessage.tsx 57.14% <ø> (ø)
src/components2/FloatingLegend/FloatingLegend.tsx 69.23% <ø> (ø)
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d32ea42...891c216. Read the comment docs.

@ohrie
Copy link
Contributor

ohrie commented Jun 13, 2022

It could be useful to use https://github.com/eps1lon/types-react-codemod for automatic migrations to the new explicit type writing for children props and more. Have you tried this already?

Oh I see, you linked it already :D

Base automatically changed from zesplus-update-2022-03 to develop July 21, 2022 12:25
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