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

should we use typescript in more modules ? #7

Open
rom1504 opened this issue May 29, 2023 · 18 comments
Open

should we use typescript in more modules ? #7

rom1504 opened this issue May 29, 2023 · 18 comments

Comments

@rom1504
Copy link
Member

rom1504 commented May 29, 2023

This has been discussed many times and the consensus is NO:

  • it requires a build step
  • it does not improve code much for PrismarineJS style code
  • it increases complexity for new contributors
@rom1504 rom1504 transferred this issue from PrismarineJS/prismarine-meta Jun 3, 2023
@rom1504
Copy link
Member Author

rom1504 commented Jun 29, 2023

There are some existing TS modules such as https://github.com/PrismarineJS/mineflayer-collectblock though

@rom1504
Copy link
Member Author

rom1504 commented Jun 29, 2023

Let's vote here. Should we try to convert some modules to typescript, for example prismarine-block and prismarine-item ?
If yes vote with up thumb, no with down thumb

@Pandapip1
Copy link

Pandapip1 commented Jan 11, 2024

The only point I've seen against TS is the following:

My opinion is still that it's bad as it encourages big projects instead of small well defined packages
- @rom1504 (PrismarineJS/flying-squid#323 (comment))

This isn't true. You can use any package structure that JS can. TS is just a superset of JS features that, when compiled, is compatible with JS.

It's extremely useful if you know how to use it. I'm not sure what we can count as proof, I run another project that is on ts and it saves me a lot of time so I can implement things faster. Let's see a pr...
- @zardoy (PrismarineJS/flying-squid#323 (comment))

This. This. And this. If you haven't tried TS yet and have been using only vanilla JS, spend a day using it. It changes nothing, yet it changes everything. I would wax lyrical for hours about how much better TS is than JS, but I don't think that's particularly productive in this context.

But yea. Compile-time errors are just better than runtime errors. And it's not hard to use TS. And it's hard not to use TS.

@rom1504
Copy link
Member Author

rom1504 commented Jan 11, 2024

Please send one project using typescript that does not look like a java project

@Pandapip1
Copy link

Pandapip1 commented Jan 11, 2024

Please send one project using typescript that does not look like a java project

How is that an argument against TS? Having a well-structured project hardly seems like a negative to me. But if you insist:

https://github.com/ethereum/eip-review-bot

@rom1504
Copy link
Member Author

rom1504 commented Jan 11, 2024

We are doing a project structure in PrismarineJS that is very different from the java style. That is on purpose and has been working pretty well.
Doing small packages with a flat API and organization rather than having many different levels of API with a single huge monolith.

This is important to take into account when choosing technology for PrismarineJS

@rom1504
Copy link
Member Author

rom1504 commented Jan 11, 2024

https://github.com/ethereum/eip-review-bot looks ok but it has no doc nor example so it cannot be used as is

@Pandapip1
Copy link

Doing small packages with a flat API and organization rather than having many different levels of API with a single huge monolith.

Oh, you mean that. That's entirely possible with TS, but is just not the way it's usually done. There's no reason why it can't be done.

https://github.com/ethereum/eip-review-bot looks ok but it has no doc nor example so it cannot be used as is

I'd be very surprised if any of the code from that project of mine would make it into PrismarineJS.

@zardoy
Copy link

zardoy commented Jan 11, 2024

Does that full TS flying squid fork look like Java project? Also p web client along with the p viewer inside is using ts efficiently. It wasn't big effort to add it to p viewer since ts plays nice with classes but allowed me to catch many bugs

@rom1504
Copy link
Member Author

rom1504 commented Jan 11, 2024

I'd be very surprised if any of the code from that project of mine would make it into PrismarineJS.

That makes sense. I was just commenting on the fact that this may not be the best example of a TS project

@rom1504
Copy link
Member Author

rom1504 commented Jan 11, 2024

Does that full TS flying squid fork look like Java project?

This one is a direct translation to TS so no, the structure seems fine.
It's not super clear to me how the TS helps there but feel free to open a PR and let's discuss specifics there

@Pandapip1
Copy link

Pandapip1 commented Jan 11, 2024

FWIW I'm currently converting prismarine-auth to typescript. In the process of converting, typescript threw an error that directly pointed to a bug, which I was able to immediately fix. This has happened twice so far. If the project had been TS from the beginning, those bugs wouldn't have ever existed!!!!

@Pandapip1
Copy link

I just saw the following points. I'd like to quickly discuss:

it does not improve code much for PrismarineJS style code

The point of TS is that it doesn't affect the code at all.

it increases complexity for new contributors

This could be considered to be true for inexperienced contributors unfamilliar with TS syntax. However, it significantly decreases complexity for downstream users AND decreases complexity for more experienced contributors.

@SilkePilon
Copy link

SilkePilon commented Jan 11, 2024

This could be considered to be true for inexperienced contributors unfamilliar with TS syntax. However, it significantly decreases complexity for downstream users AND decreases complexity for more experienced contributors.

so why make it harder for new contributors to help with the project. Having yet another layer of learning Typescript isn’t going to help. The only different will be that some users who know typescript will have a bit better contribution experience. This doesn’t say i hate typescript, in fact i like it and uses it regularly. I don’t think its good to have 2 types of languages for 1 “project”. Using more typescript will eventually split the project in 2. Typescript can be a good option but only if we fully switch to it in all repository’s

@Pandapip1
Copy link

so why make it harder for new contributors to help with the project

Sorry to be terse, but -- did you actually read my comment? I explicitly said: "However, it significantly decreases complexity for downstream users AND decreases complexity for more experienced contributors."

I don’t think its good to have 2 types of languages for 1 “project”. Using more typescript will eventually split the project in 2. Typescript can be a good option but only if we fully switch to it in all repository’s

I agree in one sense - I would rather that prismarine use 100% typescript. However, I disagree that this will cause fragmentation. TS and JS are interoperable.

@SilkePilon
Copy link

Sorry to be terse, but -- did you actually read my comment? I explicitly said: "However, it significantly decreases complexity for downstream users AND decreases complexity for more experienced contributors."

I agree that typescript is better and probably better for documentation but having 2 different languages is still weird. I know that JS and TS can be used together but I think a 100% or 0% is better. https://youtu.be/5ChkQKUzDCs?si=Kb2Nd8KoulTGVGlS I think this video explains it better than i do

@Pandapip1
Copy link

Pandapip1 commented Jan 11, 2024

I've seen that video. But the crucial thing is that turbo is still keeping its .d.ts files. I also disagree with the maintainer's point that a compile step that alerts you of errors is bad. I'd rather fix a dozen clearly benign but clearly indicated type errors than try to fix a single obscure error caused by an incorrect type

@SilkePilon
Copy link

@rom1504 what do you think about this?

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

4 participants