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

Improve Typescript support #7334

Open
3 tasks done
sadortun opened this issue Apr 10, 2021 · 14 comments
Open
3 tasks done

Improve Typescript support #7334

sadortun opened this issue Apr 10, 2021 · 14 comments
Labels
type:feature New feature or improvement of existing feature

Comments

@sadortun
Copy link
Contributor

sadortun commented Apr 10, 2021

New Feature / Enhancement Checklist

Current Limitation

Continuation of #7287

Current Typescript definitions are out of sync with this repository. Manually updating them in @types/parse is not a viable solution if you want to support the future semantic versioning and gradual rollout of new features, and deprecations.

I see two options to fix this issue. First one is a small incremental step that will solve the issue in the short term, and second option that will enable parse-server to be Typescript compliant.

Feature / Enhancement Description

1. Incremental solution / short term

This is probably something that will take about an hour to put in place and have no backward compatibility issues. I can make a PR in the next days, and then update missing/invalid definitions.

DefinitelyTyped repo is good when the main project doesn't support typescript or types are contributed from a separate entity, but I think in the case of Parse we should have types definitions moved back to the intro parse-server and JS SDK repo.

In addition, expecially considering the plan to include a deprecation strategy that will span on multiple branches (stable, beta, next).

Theses types definitions needs to follow the branch and release cycles.

Its natively supported by package.json and Typescript. I don't see any drawback to dooing this. Or any impact on non-typescript users

This will also ensure that new PR that adds or remove features will have matching Typescript definitions. Otherwise, I have no idea how you can synchronize merged PR additions with the type definitions.

As of now, there is still a lot of incorrect/missing Typescript definitions in Parse-server.

Having theses types in this repo would solve so many issues. Otherwise @types/parse will never be in sync with the actual code.

I can submit a PR about this in the next days, and also fix multiple broken definitions.

As of @types/parse you can leave it in this current state, and in the next releases notes add a deprecation notice to mention the definitions are now part of the project.

2. long term Typescript support

I'm leaving this here, since it's the logical path forward. But we may want to track this in a separate issue.

To make parse-server work with typescript, we first need to rename all js files to ts ( see this issue microsoft/TypeScript#35470 )

After that, we need to evaluate the extent of the changes to make the project 100% typescript.

This is a more complex process that may have backward incompatibilities, and may also cause issue for existing PRs and forks.

Example Use Case

Alternatives / Workarounds

@ts-ignore all errors. Not very practical ....

3rd Party References

PS: I don't want to rush anything, I'm just putting ideas out there :) I wish everybody a great weekend : 🍺

@dblythy
Copy link
Member

dblythy commented Apr 10, 2021

@sadortun I hope you have a good weekend too, your suggestions are really valued and I look forward to seeing your contributions and using them myself!

If, for example, ParseCloud.js was converted to typescript, and all the types were laid out there, would this enforce types when using Parse.Cloud? Would this remove the need for DefinitelyTyped?

I’m assuming that in order to remove the need for DefinitelyTyped, the JS SDK too would have to be in typescript. Is that a fair assumption?

I’m not overly familiar with how typescript works and I’ve never created a .ts file, I just use VSCode and not having completion for the validators was annoying.

@sadortun
Copy link
Contributor Author

Hi @dblythy I'm not a Typescript expert but I'm pretty sure we can do this simply without touching the current codebase.

My idea is to simply copy @types/parse into parse-server repo under /types folder and refer it in package.json

See types field.
https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html#including-declarations-in-your-npm-package

That way we could simply add/update definitions in this repo using PRs and ensure contributors PRs also update the types.

From what I understand, types definitions are complementary to the actual JavaScript code and should not interfere with non-typescript users.

We could also split up definition between server and JS SDK so everybody manage their own types :)

@++

@mtrezza
Copy link
Member

mtrezza commented Apr 10, 2021

I think there are 3 separate TS issues:

  1. Move TS definitions from DefinitelyTyped to Parse Platform (still manual update)
  2. Auto-generate TS definitions as part of CI (no manual update)
  3. Convert repo to use TS

2 requires 1, but 3 is independent.

For 1, depending on where the TS definitions should be (Parse JS SDK, Parse Server repo, split into both, or dedicated repo), this issue may actually not belong into Parse Server repo.

To scope this issue more clearly, I understand that this issue focuses on 1 only? Couldn't we skip 1 and already focus on 2?

@sadortun
Copy link
Contributor Author

sadortun commented Apr 10, 2021

Hi @mtrezza good breakup of things.

For 1, depending on where the TS definitions should be ...

Indeed, the same process should be done also in JS SDK. But from what I see from using TS, most of the remaining issues in definitions are in server

Also, I think it can be an easier to start with server since the scope is much more limited. And codebase smaller, and if we introduce a type problem, it will affect less users if it's in server

this issue may actually not belong into Parse Server repo.

I think types should be stored in their respective repositories. (js sdk and server)


2 requires 1, but 3 is independent.

From what I understand,

  • 2 (most likely) need 3 first.
  • 2 is an improvment over 1.

Since GitHub doesnt have threaded issues, I suggest that we keep this issue focused on (1) and expand later on other issues.


1

Would you be OK in having the types getting moved to the server ( and js sdk later) ?

Of course, the end goal is to generate them automacttly, but This can be done a bit later.


2 && 3

I'm putting theses references for later use :

I'll try this, and if succesful I'll open a different issue for (2).

  • convert flow to ts. ( To a tmp folder
  • generate definitions from generated typescript.

If this work, this may give us more input if (3) is doable ( I'll open another issue for that too )

@mtrezza
Copy link
Member

mtrezza commented Apr 10, 2021

2 (most likely) need 3 first.

Can't we just generate the TS definitions from JSDoc? I assumed all we needed was maybe some JSDoc refinement and setup the generation process.

https://www.typescriptlang.org/docs/handbook/declaration-files/dts-from-js.html

This set up means you can own the editor experience of TypeScript-powered editors without porting your project to TypeScript, or having to maintain .d.ts files in your codebase. TypeScript supports most JSDoc tags, you can find the reference here.

Every minute we spend on manual TS is lost time, if we replace it with auto-generation anyway. If we generate from flow then we reinforce a dependency on flow which we actually want to get rid of / replace with TS.

I think we need to focus on our JSDoc quality (also for our own docs) and generate the TS files. We may well start with Parse Server as you suggested, which has less TS relevant code.

@sadortun
Copy link
Contributor Author

sadortun commented Apr 10, 2021

@mtrezza As mentioned before, it doesn't seems like there is a way for Typescript to parse JS files that contains non-js code like in server that contains flow definitions. Typescript just dies......

microsoft/TypeScript#35470

If you find a way around this, it would be great.

Every minute we spend on manual TS is lost time, if we replace it with auto-generation anyway.

Yes I agree, BUT, to be realistic, current typescript declarations do about 95% of the job OK. The remaining 2-3 hours needed to fix the last things is a pretty small effort, Im fine spending this time to have good Typescript completion in our projects.

In comparison, generating definitions from either JSDoc or Flow is a massive undertaking, that probably require fixing hundreds of errors...


EDIT: Just to clarify, (1) is something we are going to do anyway in our forks, so the real question is, do you want a PR about this ? We can always investigate further for (2) and (3) after.

In any cases, from either (1), (2) or (3), definitions needs to be in the js sdk and server projects. Having theses definitions in the project sooner will only make more people move away from @types/parse and transition to in-project definitions. To put it a little bit crude, i dont think any user really care if the definitions are manually created or generated, but everybody cares about incorrect or missing definitions. And this is what i want to do here. Just fix/patch them for the short term.


I'm leaving the whole discussion of regenerating the definitions for a separate issue. I don't have time now to embark on this kind of large/complex task right now 😅 .

It would be a nice goal to have for maybe a year from now / v6.0.0

@mtrezza
Copy link
Member

mtrezza commented Apr 10, 2021

Why don't we just strip the flow annotations?

Run npm run types in the PR below and all types are generated (2).
The compiler accepts JS and TS files and we can gradually implement (3) at some point.

@sadortun
Copy link
Contributor Author

@mtrezza yes, that's an option too. It all depends on how confident you are about JSDoc.

But if the mid/long term goal it to transition to Typescript, I think that using a tool like flow-to-ts might be a simpler approach since it will allow to do both the generation of definitions, and typescript code to merge later.

flow-to-ts would allow to find issues with existing code and fix issues in advance, and at some point, transition to Typescript.

If you only use JSDoc for generating definitions, you are probably going to have to use flow-to-ts later anyway when you'll want to move to Typescript, and in not sure if at that point JSDoc will be of any use to perform the transition.

This is just my 2 cents. More tests are probably needed to figure out the right way to go.

@mtrezza
Copy link
Member

mtrezza commented Apr 11, 2021

I suggested that because it decouples flow from ts via JSDoc, to allow an easier transition to ts (3).

Can you make a PR and create the definitions using flow-to-ts like I did in the PR?

I think whatever we choose, we need a proof of concept in a PR for (2) and a strategy how to achieve (3).

If we choose flow-to-ts for (2), then we should know how to do (3) with a repo that is mixed TS, Flow and JS, with one depending on the other, that could be a challenge. Not sure if I bring across what I mean to say?

Cleaning up JSDoc for (2) provides a basis for switching to TSDoc as part of (3). JSDoc is always useful and required to build our docs files. JSDoc is also what the IDE displays to the developer. But you are correct, once we switch to TSDoc, the types in JSDoc will be removed.

@dblythy
Copy link
Member

dblythy commented Apr 11, 2021

@mtrezza
Copy link
Member

mtrezza commented Apr 11, 2021

The discussion is about 3 or 2?

If all relevant types are in the JS SDK, then maybe for Parse Server only 3 is relevant but for the JS SDK also 2? I guess for Parse Server repo typed Parse Server options and Cloud Code would be most relevant for developers, the rest of officially exposed methods, interfaces and properties seems to be in the JS SDK?

@mtrezza mtrezza reopened this Apr 11, 2021
@mtrezza
Copy link
Member

mtrezza commented Apr 11, 2021

I suggest we keep this open, as the issue itself still exists?

@mtrezza mtrezza changed the title Improve short term Typescript support Improve Typescript support Apr 11, 2021
@mtrezza mtrezza added type:feature New feature or improvement of existing feature and removed type:improvement labels Dec 6, 2021
@mtrezza mtrezza mentioned this issue Nov 2, 2022
3 tasks
@agnostic-coder
Copy link

any update on this? landed here after trying to access the options types in new ParseServer(options) . It is unfortunate to go back and foth to the docs for these options while it can be integrated using jsdoc comments.

@mtrezza
Copy link
Member

mtrezza commented Oct 12, 2023

@sadortun Could we apply the same approach here like we did in the JS SDK in parse-community/Parse-SDK-JS#1950? And maybe start with the Parse Server options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or improvement of existing feature
Projects
None yet
Development

No branches or pull requests

4 participants