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

Add Typescript types #4505

Closed
fzaninotto opened this issue Mar 10, 2020 · 28 comments · Fixed by #5291
Closed

Add Typescript types #4505

fzaninotto opened this issue Mar 10, 2020 · 28 comments · Fixed by #5291

Comments

@fzaninotto
Copy link
Member

fzaninotto commented Mar 10, 2020

Context

We want to migrate react-admin to TypeScript and release types with the library.

That doesn't mean that you won't be able to use react-admin if you're not using TypeScript. React-admin will still publish both JS files (as today) and .d.ts files (ignored by non-typescript users). So if you don't use TypeScript, that won't change anything for you.

We've started a migration effort a year ago. As of writing, 70% of the codebase is in already TypeScript. We use these types internally during development. We've only released the types for ra-core publicly.

We're working on migrating the rest of the codebase. It's a long and tedious job. It's not the core team priority. Your help is welcome.

Risk

We don't want to emit bad types, because it would break existing TS applications. Therefore, we need to test the types thoroughly.

Roadmap

Migrating the demo should be the priority, as it allows us to validate our types internally before publishing them.

  1. ✔️ Migrate all .js files to .ts files in the example demo (https://github.com/marmelab/react-admin/tree/master/examples/demo).
  2. ✔️ Migrate all ra-ui-materialui JS files to TypeScript. Publish the compiled types in the next ra-ui-material-ui release.
  3. ✔️ Convert the rest of react-admin dependencies (ra-language-english, etc) to typeScript, publish their types
  4. ✔️ Locally, emit the types from react-admin in development. This is when things will break. Test the demo, test on as many existing TS apps as possible. Once all the types are checked, and only then,
  5. ✔️ Release react-admin types officially
  6. ???
  7. Profit

All these changes must be done on the next branch (master is only for bug fixes).

How you can help

If you know and understand the react-admin internals, and if you are an experienced TypeScript developer, please help us migate .js files to .ts in the order described above.

This is not an easy task. If you don't have enough knowledge of the react-admin internals, then please leave it to people more knowledgeable. Otherwise, the core team will spent more time answering your questions than doing the migration themselves.

What to do in the meantime

Some community packages for react-admin types can be found here and there. They are outdated. Disable TypeScript checking on the react-admin modules.

@josephktcheung
Copy link
Contributor

@fzaninotto thanks for writing this. I will split my last PR on ra-ui-materialui #4448 into smaller ones for review, but I think I’ll take a stab at Point 1 migrating demo to ts first.

@iamstiil
Copy link
Contributor

Happy to help :)

@cbnsndwch
Copy link

@fzaninotto I just came across this repo while searching for a React front-end framework for an app I'm building. I am of course not familiarized with it yet, but I have 5+ years experience with TS. I'd be happy to help.

@mayteio
Copy link
Contributor

mayteio commented Mar 19, 2020

I'd love to help out with ra-langauge-english, ra-language-french and ra-data-graphql.

For the language packages - they are simple objects with the same structure. Would it make sense to create the translation map type in ra-core so it can be shared between them?

I've been working with ra-data-graphql extensively while developing ra-aws-amplify and feel very familiar with it. I'm also very comfortable with TS.

@fzaninotto
Copy link
Member Author

Yes, the messages type should be in ra-core. Just remember that developers can add their own keys outside of the 'ra' namespace.

@mayteio
Copy link
Contributor

mayteio commented Mar 23, 2020

Thanks @fzaninotto. Can they add their own keys inside the namespace?

@djhi
Copy link
Collaborator

djhi commented Mar 23, 2020

Can they add their own keys inside the namespace?

Yes, nothing prevents it. For example, I often add my own actions inside ra.actions. Same for validation.

@mayteio
Copy link
Contributor

mayteio commented Mar 25, 2020

All good. Is there anything preventing you guys from releasing types for individual packages? ra-ui-materialui is typed, for example, though it doesn't expose the types. This is really handy for things like creating custom fields & inputs, i.e. InjectedFieldProps, FileInputProps, etc.

Edit: Ah, I just see it's on the roadmap. Only half of it or so is migrated.

@gjolund
Copy link

gjolund commented Apr 7, 2020

This is awesome news.

The catch 22 of typescript is that it makes migrations and refactoring so much easier, but you have to refactor and migrate existing js into ts to get there first...

Best of luck and thanks for the amazing product and great support.

@mridulgogia
Copy link

Can I work on converting ra-data-graphql-simple package?

@fzaninotto
Copy link
Member Author

Sure, go ahead!

@glebsts
Copy link

glebsts commented Apr 17, 2020

Hello,
Just looked at RA demo and decided I'd like to use it in our next projects, started with PoC app and discovered that react-admin has no types, then found this thread. May the Force be with you, guys. I wish I could help but I don't know internals of RA. Any beta channel where I could try to use something and see if it works?

@eordano
Copy link

eordano commented May 19, 2020

This is great news. Big fan of RA; great to see this implemented!

@fzaninotto
Copy link
Member Author

Thanks to an awesome work from several community developers, we've now reached the first step in the roadmap: the example demo is now 100% Typescript. Let's continue this collective effort and focus on the ra-ui-materialui types.

@ash-r1
Copy link

ash-r1 commented Jul 22, 2020

I found that ra-core is using the name Record on the master branch.

export interface Record {

It conflicts with the same naming UtilityType which provided by TypeScript own. Can you change it?
https://www.typescriptlang.org/docs/handbook/utility-types.html#recordkt

@Tymek
Copy link
Contributor

Tymek commented Jul 22, 2020

@ash-r1 could You change it? Looks like a good-first-issue to me 😉

On a serous note, it should be possible to use

import type { Record as AdminRecord } from 'ra-core'

I'm guessing here, so feel free to open an issue on that. I might volunteer to fix it.

@cubabit
Copy link

cubabit commented Aug 20, 2020

Really looking forward to this being complete.

Is there an issue tracker or list of components that need updating as I would like to help but I do not know what anyone else is working on?

I am interested in converting ra-data-graphql

@gjolund
Copy link

gjolund commented Aug 21, 2020

@fzaninotto I don't know if you have seen this tool provided by airbnb, but I have used it recently to migrate some legacy js projects to ts and had a lot of success.

https://github.com/airbnb/ts-migrate

@fzaninotto
Copy link
Member Author

@cubabit please proceed with ra-data-graphql, it would help a lot!

@austinrivas yes, seen this, and it doesn't replace hand-crafted types. Publishing components with any as proptypes isn't much better than not publishing types at all IMO.

@gjolund
Copy link

gjolund commented Aug 24, 2020

@fzaninotto I agree 100%, I have been using it as an initial step and adding types after I have run my test suite and confirmed there are no regressions.

It's not perfect but it does save some tedious and error prone copy pasting for me.

@leoafarias
Copy link

Hopefully this is not off topic, but validated this tool and seems like it would be an asset to help bring support to typescript a bit faster across multiple packages.

https://github.com/airbnb/ts-migrate

@fzaninotto
Copy link
Member Author

React-admin now emits TypeScript types - this task is done 🥳 . Starting with 3.9.0, you will have IDE autocompletion in react-admin projects, and a stricter syntax check if you use TypeScript. Of course, you can still use react-admin in pure JS, without TypeScript.

These types aren't complete or perfect, and any TypeScript app using react-admin will probably reveal bugs in our types. That's why I've released version 3.9.0-beta.1 in the npm next channel: We need volunteers to test these types and report any problem they find in real-world usage by opening a new issue here.

Please don't open issues for missing types yet - only the ones that break current code.

Thanks in advance!

@crjacinro
Copy link

Hi. Any timeline for the official release with typescript? Thank you.

@fzaninotto
Copy link
Member Author

@crjacinro TypeScript support was shipped 6 days ago. We've announced it in various channels:

I'm interested to know how else you expected the news to be announced.

@alikondie
Copy link

Hi, any news on ra-data-grarphql and ra-data-grarphql-simple, I would be glad to help

@djhi
Copy link
Collaborator

djhi commented Feb 10, 2021

Hi @alikondie, if you see anything that could be improved, please help :)

@adamwdennis
Copy link

adamwdennis commented Feb 18, 2021

Hi @alikondie -- we're also looking for Typescript types for ra-data-graphql and ra-data-graphql-simple, but have no experience in this process. Do you have any suggestions on a guides for this process? And any idea of time it may take to do that? Thanks!!

image

@alikondie
Copy link

Hi @adamwdennis I haven't worked with it either, I just started looking at it, so I don't have any ideas.

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 a pull request may close this issue.