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

Convert demo example into typescript #4516

Merged
merged 28 commits into from
May 7, 2020

Conversation

josephktcheung
Copy link
Contributor

@josephktcheung josephktcheung commented Mar 12, 2020

This PR will convert the following files to ts:

  1. dataProvider/*
  2. fakeServer/*
  3. i18n/*
  4. routes.js
  5. themeReducer.js
  6. visitors/Aside.js
  7. orders/MobileGrid.js
  8. orders/NbItemsField.js
  9. orders/OrderEdit.js
  10. orders/OrderList.js

For others who want to help finish converting demo example into typescript, please create PRs for following parts:

@josephktcheung josephktcheung force-pushed the feature/demo-ts branch 2 times, most recently from 51ad46c to 5097061 Compare March 12, 2020 15:51
@fzaninotto
Copy link
Member

You're going too fast in the wrong direction: adding any types isn't helping. We want to use the Demo example to validate the react-admin types. Your changes disables type checking on the demo.

@josephktcheung
Copy link
Contributor Author

@fzaninotto No worries, I'll change those any type to correct types.

@josephktcheung
Copy link
Contributor Author

josephktcheung commented Mar 13, 2020

@fzaninotto I've removed most of the quick and dirty any types. Please take a look

@MohammedFaragallah
Copy link
Contributor

@josephktcheung how is it going I would like to help with this.

and let's add a reference #4505 to avoid new duplicates

* next: (179 commits)
  Update UPGRADE.md
  Fix AutocompleteInput not accepting 0 as a value
  Fix AutocompleteInput empty suggestion item height
  fix: default app loading is not not center vertically - 4302
  Fix ra-data-graphql-simple GET_MANY_REFERENCE target filter
  Moving FormHelperText above previews element
  Fix: css typo haight -> height
  v3.4.1
  Prepare changelog for v3.4.1
  review
  Update docs/List.md
  Update docs/List.md
  Update docs/List.md
  Fix tests
  Remove unused var
  Fix types
  Fix the desktop match
  Open the sidebar by default on desktop
  Don't force opening the sidebar on desktop
  Add section about filters in List documentation
  ...
@josephktcheung
Copy link
Contributor Author

josephktcheung commented Apr 28, 2020

Hi @MohammedFaragallah thanks for helping. I'd like to split up the work so that react-admin team can review each PR independently. Apart from what I've done in this PR, components in Products, Reviews and Segments are not migrated to typescript yet. Therefore, I'd suggest you and other contributors to pick 1 of them and create a separate PR.

Besides, I'd also like us to review each other's change so that the interface is written in a consistent manner. What do you think?

I've noticed that you created #4725 which also refactored example orders to typescript. I'll take a look tonight and pick up what's better and update my PR.

@MohammedFaragallah
Copy link
Contributor

@josephktcheung good plan I'll review your PR and start with Products the same way you did here to maintain consistent migration.

examples/demo/src/dataProvider/graphql.ts Outdated Show resolved Hide resolved
examples/demo/src/ra-language-english.d.ts Outdated Show resolved Hide resolved
examples/demo/src/themeReducer.ts Outdated Show resolved Hide resolved
examples/demo/src/types.ts Outdated Show resolved Hide resolved
? JSX.IntrinsicElements[C]
: {};

export interface DatagridProps<RecordType = Record>
Copy link
Member

Choose a reason for hiding this comment

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

👍 This one and the following will serve as a good base for the ra-ui-material-ui types.

examples/demo/src/types.ts Show resolved Hide resolved
examples/demo/src/orders/OrderList.tsx Outdated Show resolved Hide resolved
examples/demo/src/orders/OrderList.tsx Outdated Show resolved Hide resolved
@fzaninotto fzaninotto merged commit 206ba77 into marmelab:next May 7, 2020
@fzaninotto
Copy link
Member

Awesome, thanks!

@fzaninotto fzaninotto added this to the 3.6.0 milestone May 7, 2020
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.

None yet

4 participants