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 rewrite? #334

Closed
ctavan opened this issue Oct 14, 2019 · 19 comments
Closed

TypeScript rewrite? #334

ctavan opened this issue Oct 14, 2019 · 19 comments

Comments

@ctavan
Copy link
Member

ctavan commented Oct 14, 2019

@defunctzombie suggested in this comment that the node-uuid module could be re-written in TypeScript.

What would be the benefits of such a rewrite over modern JavaScript?

@broofa
Copy link
Member

broofa commented Oct 14, 2019

It should be noted that the reason for this is to simplify support of ES modules in browsers and Node, not because TS is particularly well-suited to generating UUIDs per se.

I'm typically not a fan of Typescript for OpenSource projects. It complicates the toolchain and makes projects less approachable.

@defunctzombie
Copy link
Collaborator

(opinion) type information has made my development more pleasant and proficient - but I also understand not wanting to take on the additional build step or barrier for other developers. Typescript has let me use one toolchain to build rather than collecting a bunch of babel plugins like I would need to do before. The API surface is the uuid module is sufficiently small and often "obvious" in a way that it itself won't benefit all that much. Like with most things - it is just preference. I've found that where I use to favor plain JS and no build step - that with new tooling the build "step" doesn't get in the way of my iteration.

Practically - I don't consider my comment of porting to TS as something that would actually happen but threw it out there anyway - hopefully without it becoming a distraction to your work.

@ctavan
Copy link
Member Author

ctavan commented Oct 14, 2019

Thanks! I still think that, given the broad adoption of TypeScript especially in application development, offering useful type definitions through @types/uuid should be a priority of this repository as well and I will work on that (#328).

For TypeScript users this should hopefully already provide most of the benefits.

@dstaley
Copy link

dstaley commented Oct 16, 2019

As a TypeScript user who uses the node-uuid package, my main desire is having types included with the package as opposed to downloading the @types/uuid package. I would prefer the library be written in TypeScript since that guarantees that the types match the behavior of the library, but I completely understand the hesitation there. I wrote a RFC for a TypeScript migration for the libraries I work on, and all of its points are equally applicable here. I'm actually of the opinion that TypeScript encourages outside contributors since it gives another layer of checks over their code. Think of it like an extra set of unit tests. In Kinto we use TypeScript to generate an ES-modules build and a CommonJS build, and then use Rollup to generate a browser-friendly ES-modules build alongside a traditional UMD build.

I'm also a big fan of using the TypeScript compiler in lieu of Babel even in pure JavaScript projects. TypeScript's inference is pretty great, so it can catch some errors even without any types. You can improve the quality of the checks by adding JSDoc formatted comments.

@sam-s4s
Copy link

sam-s4s commented Oct 29, 2019

Yes, having types included would be great. Also an important request - please make sure that @types/node is not being pulled in. It really ruins a project. I can't use the @types/uuid package at present because it pulls the node types in, and then you can't use things like setTimeout properly, because the non-browser, node versions of the types get pulled into your project, creating chaos :(

@broofa
Copy link
Member

broofa commented Dec 4, 2019

Let's close this - I think this is covered by #328

@ctavan
Copy link
Member Author

ctavan commented Dec 5, 2019

OK, we'll first make sure to get the typings in order, see #328. Should not matter if the uuid code base itself is written in TypeScript or not as long as we provide useful typings.

@ctavan ctavan closed this as completed Dec 5, 2019
@jamiehaywood
Copy link

I know this was closed almost 2 years ago, but I wanted to see if there had been any change in thoughts around the TS rewrite? I think it would be an awesome step forward for developer experience using this library, as mentioned above.

@LinusU
Copy link
Member

LinusU commented Sep 13, 2021

I personally love TypeScript, but still doesn't think that it's the right fit for this project. Since the codebase is so simple, I don't think that there is that much to be gained by switching to TS.

If anything, I think that we should consider dropping both Babel & Rollup and only ship an ESM version in the next major version. All the currently supported versions of Node.js now supports ESM, and not having any build step at all I think is something that makes the setup even more simple, which makes the project more approachable for contributors! 🚀

@piranna
Copy link
Member

piranna commented Sep 13, 2021

I think that we should consider dropping both Babel & Rollup and only ship an ESM version in the next major version.

Agree.

@dstaley
Copy link

dstaley commented Sep 13, 2021

I think that we should consider dropping both Babel & Rollup and only ship an ESM version in the next major version.

I would caution against this since the ecosystem hasn't caught up to ESM, and publishing an ESM-only package as low-level as uuid can cause significant breakage in applications (in fact, you're unable to synchronously import ESM-only packages in CJS environments like node). If uuid does decide to move to ESM-only, I'd strongly recommend maintaining the current major version with features/bug fixes until ESM support is more wide spread.

@broofa
Copy link
Member

broofa commented Sep 14, 2021

All the currently supported versions of Node.js now supports ESM

This may be true for nodejs.org's support matrix, but at this time we (uuid) still support node 8+ and IE 11.

It's important to remember that the webpack and rollup builds were added to allow for CI testing (in both node and in the browser) which was one of the more important improvements to the stability and quality of this project.

@piranna
Copy link
Member

piranna commented Sep 14, 2021

we (uuid) still support node 8+ and IE 11

But they don't have support from their maintainers... Why we should support them?

@LinusU
Copy link
Member

LinusU commented Sep 14, 2021

All the currently supported versions of Node.js now supports ESM

This may be true for nodejs.org's support matrix, but at this time we (uuid) still support node 8+ and IE 11.

Yes, my comment was a bit off since we don't actually use rollup to produce the build, my bad.

However, we currently ship three builds in the final package: CommonJS, ESM for Node, ESM for browsers. My proposal is to drop the CommonJS build, since e.g. IE 11 doesn't support CommonJS either.

Not saying that there is any stress in doing this, just that it would be nice to do it the next time we release a major release/drop support for older versions of Node.js ☺️

@Bessonov
Copy link

I know this was closed almost 2 years ago, but I wanted to see if there had been any change in thoughts around the TS rewrite?

Coming exactly for this reason here. Personally, I don't see any disadvantage in using TypeScript, especially for relatively small projects. Incremental builds and webpack 5 caching are great for DX. I hope the time will come and all runtimes would support ts natively.

This may be true for nodejs.org's support matrix, but at this time we (uuid) still support node 8+ and IE 11.

Well, then you support a very bad practice. You support data breaches. You delay support of ESM, because... ? This is the way.

I hope you will change your mind.

@piranna
Copy link
Member

piranna commented Oct 20, 2021

I hope the time will come and all runtimes would support ts natively.

If that happens some day, that will be a really sad day. Future is optional types inside Javascript, when that day arrives, Typescript will be dead.

@Bessonov
Copy link

@piranna

Well, you would be happy with a mediocre subset re-implementation of great superset? Because ...? It's like IndexedDB instead of sqllite. That put people into depression. That produce wars. That kills kittens. That truly hurts.

@piranna
Copy link
Member

piranna commented Oct 20, 2021

I've used both IndexedDB and SQLite, they serve for different purposses. Please learn to use a tool and why it works that way before critizise it without knowing, and go to troll elsewhere.

@uuidjs uuidjs locked as too heated and limited conversation to collaborators Oct 20, 2021
@defunctzombie
Copy link
Collaborator

This issue is closed because the current maintainers are not pursuing or entertaining this effort at this time. If someone wants to come along and move @types/uuid into this module via a PR or propose a typescript version that is up to them to consider contributing and then making the case for their new version over the status quo.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants