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

[Task] Update to React 18 #482

Open
maxcao13 opened this issue Jul 15, 2022 · 3 comments
Open

[Task] Update to React 18 #482

maxcao13 opened this issue Jul 15, 2022 · 3 comments
Labels
chore Refactor, rename, cleanup, etc.

Comments

@maxcao13
Copy link
Member

I haven't seen that before. I don't really have any thoughts on the matter or opinion one way or the other.

https://react-typescript-cheatsheet.netlify.app/docs/basic/getting-started/function_components/

I tend to like the explicit/static types where possible, but sometimes it isn't really necessary - and if the type signature is actually lying to you, like it is about the implicit children, then it does more harm than good.

If you'd like to go through and refactor to remove all those type declarations later then I wouldn't be opposed.

Maybe I will do a refactor in the future (the docs you linked has something for that anyways), but for now I think it may be best to stop using React.FC in -web in the future and stick with the usual

const Foo = ( {prop1, prop2}: FooProps ): JSX.Element => {
   ...
   return ...
}

so that everything is using the same and preferred style.

I'll open an issue for it, if that's okay with you anyways. This isn't a big deal by any means ^^. I'm not sure what is the best way to let everyone know though, haha.

Originally posted by @maxcao13 in #481 (comment)

@maxcao13 maxcao13 added the chore Refactor, rename, cleanup, etc. label Jul 15, 2022
@maxcao13 maxcao13 self-assigned this Aug 8, 2022
@maxcao13
Copy link
Member Author

maxcao13 commented Aug 8, 2022

I will start this after #478 is merged because of this comment on https://github.com/gndelia/codemod-replace-react-fc-typescript#codemod-replace-react-fc-typescript

(It's recommended to run your favorite formatting tool after the codemod )

@maxcao13
Copy link
Member Author

Reading this again after the recent merge and it seems like there's not really a need to refactor React.FC anymore since the implicit children problem is solved in React 18 https://reactjs.org/blog/2022/03/08/react-18-upgrade-guide.html#updates-to-typescript-definitions. Is there any push back from changing this issue to updating cryostat-web from React 17.0.2 to React 18.*?

@andrewazores
Copy link
Member

No, that sounds good, though it sounds like the upgrade may not be totally seamless judging by those release upgrade notes. We'll need to go through things carefully and make sure the changes don't cause any odd bugs. We're pretty close to the code freeze date for 2.2 release so this would need to be done quite soon to have any chance of making it in to the release.

@maxcao13 maxcao13 changed the title [Task] Refactor React.FunctionalComponent/React.FC using codemod-replace-react-fc-typescript [Task] Update to React 18 Sep 29, 2022
@tthvo tthvo moved this to Todo in 2.3.0 release Nov 3, 2022
@andrewazores andrewazores moved this from Todo to Stretch Goals in 2.3.0 release Nov 3, 2022
@andrewazores andrewazores moved this to Stretch Goals in 2.3.0 release Feb 7, 2023
@andrewazores andrewazores moved this from Stretch Goals to Pushed to 2.4.0 in 2.3.0 release Apr 6, 2023
@maxcao13 maxcao13 removed their assignment Aug 21, 2023
@tthvo tthvo moved this to Todo in 3.0.0 release Oct 3, 2023
@tthvo tthvo self-assigned this Oct 3, 2023
@tthvo tthvo removed their assignment Jan 11, 2024
@andrewazores andrewazores moved this from Todo to Stretch Goals in 3.0.0 release Feb 13, 2024
@andrewazores andrewazores moved this to Backlog in 4.0.0 release Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Refactor, rename, cleanup, etc.
Projects
Status: Backlog
Status: Pushed to 2.4.0
Status: Stretch Goals
Development

Successfully merging a pull request may close this issue.

3 participants