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

Migrate JDL to typescript #19182

Closed
1 task
mshima opened this issue Jul 18, 2022 · 14 comments · Fixed by #19856
Closed
1 task

Migrate JDL to typescript #19182

mshima opened this issue Jul 18, 2022 · 14 comments · Fixed by #19856
Labels
area: enhancement 🔧 $$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ theme: JDL v8 $500 https://www.jhipster.tech/bug-bounties/
Milestone

Comments

@mshima
Copy link
Member

mshima commented Jul 18, 2022

Overview of the feature request

JDL is quite complicated to understand and missing a few features.
We should migrate it to typescript to make it simpler and may receive more contributions.

Motivation for or Use Case

Improve developer experience.

Related issues or PR
  • Checking this box is mandatory (this is just to show you read everything)
@mshima
Copy link
Member Author

mshima commented Jul 18, 2022

What do you think @deepu105 @MathieuAA?

@MathieuAA
Copy link
Member

+1
The whole project should benefit from using TS

@MathieuAA
Copy link
Member

However, TS won't be the holy grail. It can make things a bit more understandable but we need conventions for the whole project (for instance interfaces, what actually qualifies as an internal breaking change, etc.).
This is a good solution, but it won't be enough

@mshima mshima added $$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ $500 https://www.jhipster.tech/bug-bounties/ labels Jul 18, 2022
@mshima
Copy link
Member Author

mshima commented Jul 18, 2022

However, TS won't be the holy grail. It can make things a bit more understandable but we need conventions for the whole project (for instance interfaces, what actually qualifies as an internal breaking change, etc.).
This is a good solution, but it won't be enough

Sure, can you take this ticket?

@emilpaw
Copy link
Contributor

emilpaw commented Jul 18, 2022

@mshima Which part of JDL are you talking about exactly? There are many files related to JDL. That would be probably too much to do at once.

I suggest to enable TS as a first step. Then smaller parts can be gradually migrated to TS.

@mshima
Copy link
Member Author

mshima commented Jul 18, 2022

@mshima Which part of JDL are you talking about exactly?

The entire jdl folder.

There are many files related to JDL. That would be probably too much to do at once.

Sure can be split in several PRs or even issues, but the issue is to track the entire jdl support (folder).
It's decoupled from the generators, IMO it's the best place to start typescript migration.
And then we should evaluate if we should migrate everything else and replace ESM to typescript at #19113

@pascalgrimaud pascalgrimaud removed $$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ $500 https://www.jhipster.tech/bug-bounties/ labels Jul 18, 2022
@pascalgrimaud
Copy link
Member

@mshima : I'm removing the bounty, following this https://www.jhipster.tech/bug-bounties/#how-bug-bounties-are-created

And I'm putting back the bounty, as it's totally deserved. So plz, ping Daniel, Deepu, Julien or me, if you think a ticket needs one :-)

@pascalgrimaud pascalgrimaud added $$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ $500 https://www.jhipster.tech/bug-bounties/ labels Jul 18, 2022
@MathieuAA
Copy link
Member

If you all are serious about using TS instead of JS, then we need a clear strategy that:

  • won't impact daily contributions
  • will move us toward a goal of having a good enough TS vs JS coverage
  • will actually prevent us from making mistakes that can be avoided by using TS

Thus, we have to measure those:

  • flag and count any issue where TS is impacting contributions,
  • measure the coverage and check it's going up steadily (LoC),
  • same as the first point, we need to flag and count

The last point is to check whether TS is a good fit.

@pedrolucasoliva
Copy link

I like so much prisma's schemes. Maybe it can be a source of inspiration.
https://www.prisma.io/docs/concepts/components/prisma-schema

emilpaw added a commit to emilpaw/generator-jhipster that referenced this issue Aug 26, 2022
emilpaw added a commit to emilpaw/generator-jhipster that referenced this issue Aug 26, 2022
@mraible
Copy link
Contributor

mraible commented Aug 28, 2022

We should check with the author of the JHipster JDL plugin for IDEA. I'd hate for their hard work to be impacted by this change.

https://plugins.jetbrains.com/plugin/19697-jhipster-jdl

@emilpaw
Copy link
Contributor

emilpaw commented Aug 28, 2022

Sure can be split in several PRs or even issues, but the issue is to track the entire jdl support (folder).

I already worked on migrating some files to TS in #19558. So far it seems the code migration itself will be rather straightforward. But I think it might be better to just first configure the compilation and build of the project before working on migrating the code. I started to work on the configuration here: #19559

I like so much prisma's schemes. Maybe it can be a source of inspiration.

I am not sure in what regards this might be an inspiration. The ticket is about migrating the code of JDL from JavaScript to TypeScript. The JDL itself should not change.

@MathieuAA
I agree that we need to plan a migration strategy for the rest of the project, but I think this could be discussed after this ticket in a new issue. WDYT?

@mraible
The build output will be JavaScript, so it should not be a problem.

@mshima
Copy link
Member Author

mshima commented Sep 27, 2022

@emilpaw I’ve managed to use mocha + @esbuild-kit/esm-loader. It works pretty well, I couldn’t see any performance drop.
Have problems when updating inline snapshots, but IMO is worth it.

@emilpaw
Copy link
Contributor

emilpaw commented Sep 27, 2022

@mshima I created a draft PR for what I've done so far: #19856
Interestingly the pipeline succeeds while locally some of the tests fail.

In the last few days, I had no time to work on this but I am continuing to work on it again now.

@MathieuAA
Copy link
Member

A good way to migrate the project, JDL included, could be to:

  • simply add the TS dependency in the project and some setup so that there's no change at all
  • module by module, file by file (PR by PR) change the extension to use TS and use TS concepts

This makes the migration process way easier and the review way easier too

@DanielFran DanielFran added this to the 8.0.0-beta.1 milestone Jun 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: enhancement 🔧 $$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ theme: JDL v8 $500 https://www.jhipster.tech/bug-bounties/
Projects
None yet
7 participants