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

Update 21 react examples to v18 #298

Merged

Conversation

andys8
Copy link
Contributor

@andys8 andys8 commented Aug 10, 2022

Updated react hooks examples to render in #root to avoid body warning and use new createRoot function to avoid deprecation warning.

  • Two examples had a different index.html
  • The component is now consistently mkApp :: Component {} in all examples
  • Only RoutingPushReactHooks has a different main function because of routerProvider with ReaderT
  • Tiny line break improvements (e.g. one line lets)

Examples

  • HelloReactHooks (updated in previous PR React v18 update #294)
  • BookReactHooks
  • ButtonsReactHooks
  • CardsReactHooks
  • CatGifsReactHooks
  • ClockReactHooks
  • ComponentsInputReactHooks
  • ComponentsMultiTypeReactHooks
  • ComponentsReactHooks
  • DragAndDropReactHooks
  • FileUploadReactHooks
  • FormsReactHooks
  • GroceriesReactHooks
  • ImagePreviewsReactHooks
  • NumbersReactHooks
  • PositionsReactHooks
  • RoutingHashReactHooks
  • RoutingPushReactHooks
  • ShapesReactHooks
  • TextFieldsReactHooks
  • TicTacToeReactHooks
  • TimeReactHooks

Related

Review

Actual diff (without formatting PR) can be better viewed by comparing this and the base branch

@JordanMartinez
Copy link
Owner

Things left to do here:

@andys8 andys8 force-pushed the update-react-examples-to-v18 branch from 7325132 to b7bf89f Compare August 13, 2022 01:16
@pete-murphy
Copy link
Collaborator

The component is now consistently mkApp :: Component {} in all examples

I started using Component Unit in some of the later recipes I added after seeing that's how Madeline defined Components that take no props. I think I had been using {} for empty props because I had assumed that props needed to be a record. That said, I don't think there are established idioms here, and I don't know of any reason to prefer Unit over {} so no need to change it.

@JordanMartinez
Copy link
Owner

I don't know of any reason to prefer Unit over {} so no need to change it.

Perhaps because Unit used to be an empty record, so they were the same thing at runtime?
Unit's runtime was changed to undefined because functions that returned Unit (i.e. {}) were slower than those that returned undefined. We guessed it had something to do with the allocation of an empty record that is immediately thrown away.

@andys8
Copy link
Contributor Author

andys8 commented Aug 16, 2022

I think in PureScript one can use anything as props. From a react perspective props are a key value pairs and can be represented as html attributes. Therefore the empty record/object seems a bit more idiomatic than unit, undefined or null to me.

I'd be open to changing if there is a reason to do so, but I think we're good to go as is :)

Copy link
Owner

@JordanMartinez JordanMartinez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andys8 Thank you so much for all your work on both this repo and in the Try PS repo.

@JordanMartinez JordanMartinez merged commit d7973f1 into JordanMartinez:master Aug 16, 2022
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.

3 participants