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

Typescript declarations #1617

Closed
in19farkt opened this issue Mar 8, 2018 · 14 comments
Closed

Typescript declarations #1617

in19farkt opened this issue Mar 8, 2018 · 14 comments

Comments

@in19farkt
Copy link

Hi. I want to write typescript types declarations for this library and could you please help me to plan how to do it. Does it's worth to cover version 1.4 or wait for version 2.0 and make types for its?

@fzaninotto
Copy link
Member

We've spoken about that in #250: we're not users of typescript ourselves and couldn't maintain these declarations.

I advise you to focus on next, and write the types as standalone files.

@in19farkt
Copy link
Author

Thanks. I'll start writing types for next and post them in DefinitelyTyped

@clementgarbay
Copy link

Hi @in19farkt, did you start writing types file or not yet ? I didn't see types package for this project on DefinitelyTyped

@in19farkt
Copy link
Author

Yes, I started writing types, but it's hard and time-consuming. The draft version can be viewed here. I did not make changes for a long time, because I copied these types into the project so that it was easier to change them.

@abumalick
Copy link

I think you started to use typescript, right @fzaninotto ?

Are you planning to ship types for react-admin now?

@fzaninotto
Copy link
Member

We'll ship them when they are ready and complete.

@user753
Copy link

user753 commented Jan 27, 2020

@fzaninotto I think even some basic uncomplete declarations would be very helpful for typescript users.
For example you could use typescript 3.7 and get declarations for js files before full migration to typescript.

@fzaninotto
Copy link
Member

There is a PR to update TypeScript to 3.7 (#3805), but it's not mergeable.

@Vadorequest
Copy link

@fzaninotto Please re-open this issue. There are multiple issues about TypeScript definitions, but none are open (that I could find). One should be open until the issue is actually resolved, otherwise people will keep opening new ones. So I suggest you reopen this one so that we may continue to discuss this topic here.

I'd like to explain a bit TS history, has you stated you're not familiar with it (in 2018).
DefinitelyTyped is the reference of TS typings, and it doesn't matter who adds them there. Anybody can. It's used as a fallback solution by developers when a library doesn't include/release its own typings.

The most preferred solution nowadays is to include typings from the lib itself, it's keeps things centralised and works well with new releases. This process is automated when the project itself uses TypeScript.

I'm starting with react-admin today in 2020 and there are still no typings, or quite a few:

declare module 'ra-data-graphql' {
  export const QUERY_TYPES: string[];
  type graphQLDataProvider = (
    fetchType: string,
    resource: string,
    params: { [key: string]: any }
  ) => Promise<any>;
  const buildDataProvider: (options: any) => Promise<graphQLDataProvider>;

  export default buildDataProvider;
}
declare module 'react-admin' {
  export const GET_LIST: string;
  export const GET_ONE: string;
  export const GET_MANY: string;
  export const GET_MANY_REFERENCE: string;
  export const CREATE: string;
  export const UPDATE: string;
  export const DELETE: string;
  export const DELETE_MANY: string;
  export const UPDATE_MANY: string;
}

It lacks the most important things, like component props and such. (that's what the toughest to figure out, I couldn't find a list of props from the doc, eventually found https://github.com/marmelab/react-admin/blob/master/packages/ra-core/src/types.ts which looks much better)

I see some work has been done through #3805 but was eventually closed and moved to #4375 which seems stuck, awaiting some feedback from you. But so far, nothing actionable/usable has happened, as far as I can tell (respectfully).

Also, I've noticed you've migrated most of your codebase to TS. Maybe the situation has changed and you intend to release typings yourself, bundled with react-admin library? Could you share with us what you see happening regarding TS typings, short-term? Thanks.

@fzaninotto
Copy link
Member

I've explained our strategy multiple times. Let me rephrase it here.

  • We definitely want to migrate react-admin to TypeScript and release scripts within the library
  • We've started a migration effort a year ago. As of writing, 70% of the codebase is in TypeScript. We've released the types for ra-core publicly.
  • We're working on adding types to ra-ui-material-ui and the demo example. Migrating the demo should be the priority, as it allows us to validate our types internally before publishing them.
  • It's a long and tedious job. It's not our priority. Your help is welcome.

@rvion
Copy link

rvion commented Mar 6, 2020

@fzaninotto great to hear 🙏. As for @Vadorequest , I did not came across such a clear statement before.

@fzaninotto I'm +1 on @Vadorequest 's request to keep one typescript-related issue opened with this clear statement and a small progress report

  • It would also be very usefull to provide usefull pointers and snippets to help users mitigate typings absence meanwhile in this issue (link to where to find props definitions, or user-made snippets)
  • a call to contributions would possibly help bring contributors on this

@Vadorequest
Copy link

Vadorequest commented Mar 6, 2020

Thanks @fzaninotto for the clarification, it really helps when things are clearly stated as such, for better collaboration.

Roadmap:

  1. Focus on https://github.com/marmelab/react-admin/tree/master/packages/ra-ui-materialui (add missing types) - Start from the https://github.com/marmelab/react-admin/tree/next branch
  2. Focus on https://github.com/marmelab/react-admin/tree/master/examples/demo (use updated ra-ui-materialui) to test if types work as expected - Start from the https://github.com/marmelab/react-admin/tree/next branch
  3. Release types officially

Is that correct?

Workaround:

We should re-open the issue now that the "what needs to be done" is clearer, and reference this roadmap in any duplicated issue.

Also, I suggest changing the issue's title to something more explicit like "TypeScript types/declarations - Roadmap", or similar. And edit the first response to point to this comment so that newcomers can jump right into the stuff that matters.

@fzaninotto
Copy link
Member

No, the roadmap is:

  1. Focus on https://github.com/marmelab/react-admin/tree/master/examples/demo (use updated ra-ui-materialui) to test if types work as expected - Start from the https://github.com/marmelab/react-admin/tree/next branch
  2. Focus on https://github.com/marmelab/react-admin/tree/master/packages/ra-ui-materialui (add missing types) - Start from the https://github.com/marmelab/react-admin/tree/next branch. Publish ra-ui-material-ui types
  3. Convert the rest of react-admin dependencies, publish their types
  4. Emit the types from react-admin in development. 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

As for how to circumvent the absence of types in the meantime, I'll let the community figure our recommendations - this is not something we want to work on.

I'll open and pin another issue to explain this.

@fzaninotto
Copy link
Member

Opened issue #4505 on that subject

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

No branches or pull requests

7 participants