-
Notifications
You must be signed in to change notification settings - Fork 54
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
When TypeScript? #249
Comments
No, would this unblock something for you though? We should be exporting type definitions that are accurate as it's built into the build, testing and linting toolchain here so they're not just handcrafted. So I'd be interested to know what the advantage would be to users of us doing a TypeScript conversion here. |
When something weird happens in IPLD codebase, eventually you end up going through all the relevant code. Once in a while it leads to bunch of tabs open with TS code, and multiformats (and some others) in JS, which requires quite a context switch to grok. |
I'm in favor of converting this library to typescript. Several reasons:
|
My experiences of converting the js-libp2p stack to TypeScript have been a net positive, with some exceptions as to how we release changes to interfaces in external modules. I don't think we could release changes with any real degree of confidence across so many modules without TypeScript. Obviously tests are king in all this but simple type checking has removed a whole class of bugs. Doubling down on that using JSDoc to generate types places a lot of limits on what you can do, for example, interface support isn't there so you normally end up with some typescript in your project anyway. JSDoc is also full of weird gotchas like when you use The type contortions we regularly do with JavaScript aren't possible with TypeScript, at least, it's possible if you try really hard but generally it's much less effort to just do the simple thing, then you end up with a codebase that is much easier to follow and everyone wins. Finally the type annotations proposal has reached stage 1 so this stuff might be coming anyway, we'd just get slightly ahead of the curve. |
ugh, well, I suspect @Gozala would be on board with this so that would leave me as the lone holdout in terms of who's left maintaining this stuff, so I suppose I won't be a blocker if someone wants to take on the task of converting this and making it 1:1 compatible with what we publish now. |
The import path must include the extension when publishing ESM as otherwise the module loader doesn't know how to find the file. Fixes errors like: ``` Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path. ```` This is a weakness of JSDoc types and would be caught by converting to TypeScript. Refs: multiformats/js-multiformats#249
@rvagg I believe @wemeetagain has already done the conversion here #251 |
Here are my sentiments in no particular order tl;dr While I won't fight for this, I do find context switching to be worthwhile tradeoff for:
Here is the longer version:
At the end of the day I personally care about benefits type checker brings and not about the new notation that TS team has invented. If I have to deal with compilation, I would much rather write in more proper typed language like Rust. Dealing with all the TS in JSDoc quirks
Yeah you can't define interface next to implementation. I do however think that forcing you to separate interface definition from implementation is a nudge into right direction. Our team settled on pattern where we define interfaces in a separate
That is annoying indeed and TS team seems to have no interest fixing (probably they are more focused on type annotations syntax proposal). We address above issue by not doing import typedefs, instead we just import I believe that also avoids doc generation issues.
Again jsdoc syntax is definitely not as pleasant to read / write but I'm not sure if I've encountered a case where I could not express something I needed and would be curious to know the examples. I think TS jsdoc syntax is not as well documented, but feature wise other than interface and abstract classes I don't think they're really apart. ConclusionI don't think switching to TS syntax is a clear win, at leas not until type annotations syntax extension has shipped in JS engines (and when that happens I highly suspect some TS syntax will also change or get deprecated). I would suggest carefully weighting tradeoffs before going ahead with this. Either way I'm not going to stand in they way. As an aside this might be a good opportunity to exercise community driven collective decision making |
I think we should also let @hugomrdias weave in on this, who used to advocate for TS in JSDoc, (and convinced me it was better tradeoff than TS which I used to be in favor of) yet more recently had to work with pure TS code base, unless I'm mistaken. |
I tend to agree with @wemeetagain and @achingbrain. TS is pretty much the industry standard such that its syntax is being incorporated (almost the same) into base JS with the type annotations proposal (that I am very much looking forward to!!). It will bring consistency to what most large projects already use. While it is a hurdle to initially get over, it is well worth it for both community inclusion, developer experience and modernization of syntax. +1 vote for TS. |
I personally believe Typescript vs JS+JSDoc debate should be over by now. There were so many known issues in vanilla JS, because of which Tyepscript was eventually inevitable, and not all those issues can be addressed by adding up a few JSDoc comments on top of JS code. If that were the case community would have evolved more towards it, instead of adopting the TS ecosystem. If one wants to go with the generic evolution of the ecosystem, then one must move to TS to make projects future-proof. The benefits it would bring to contributors are also not hidden a simple Google search can list thousands. That was all from the contributor's side of view, how about the consumer? What do consumers need? Ultimately three things 1) Stable library 2) Better Typesafety 3) Best Documentation. For the first two points, TS can be very helpful to achieve for later points TS can provide a better value addition with less maintainability for mismatch types. |
Please, our life is hard enough already as to voluntarily opt-out of Typescript. It's not great, but it's the best thing we have now by far |
im still very much on ts with jsdoc side, theres 0 reasons to add extra steps to an already complex js tooling situation. i subscribe to everything @Gozala said here for everyones reference here is a big project moving from ts to jsdoc ts https://devclass.com/2023/05/11/typescript-is-not-worth-it-for-developing-libraries-says-svelte-author-as-team-switches-to-javascript-and-jsdoc/ If theres any type issues with using multiformats as it is now im available to help fix it but if theres no problems that need fixing why change to a worst solution ? |
So there's been a lot of good discussion here - thanks all for contributing. My wants:
Who ultimately are the maintainers of this repo? I don't think we've been explicit about this. Looking at commits the last year (https://github.com/multiformats/js-multiformats/graphs/contributors?from=2022-06-07&to=2023-06-07&type=c ), we've had:
I know @achingbrain is supportive of this work (as he approved the PR) and my understanding is that @rvagg and @Gozala aren't strictly against it but want to make sure this is really going to making things better in some way (e.g., help consumers, increase contributions, reduce maintenance for maintainers, increase the number of maintainer hours with more humans involved). In this thread we have had some signal from consumers that this would be beneficial:
Ways I can see to unblock the decision-making here include:
|
Per 2023-06-06 js-libp2p maintainers calls, @wemeetagain will add a diff of generated outputed types as well as do a drop in replacement with #251 into the node_modules folder and verify everything works (in addition to the already passing tests).
We can have this convo in the upcoming 2023-06-14 js-libp2p maintainers call. Welcome others to join as well. |
Update: @achingbrain and @wemeetagain are ok with taking on maintenance of this repo. It's mainly used in the libp2p (and Helia!) domain so it makes sense for the js-libp2p team to steward this. So the answer to this
is yes. Should we add this repo to the js-libp2p triage/maintainers call then @rvagg @BigLep ? How do you propose we handle responsbilities there. Having said that all that, would you be ok with approving the changes proposed in #251 @rvagg |
This isn't quite correct, the point of js-multiformats is that it sits at the core of all the PL stack for JavaScript users. It gets used by everyone regardless of whether they touch libp2p (e.g. in the 4.5 years I've been with PL, I've written very little JS code that touches libp2p directly, and not a whole lot more that touches it indirectly!). Even within PL there's the web3.storage team that builds on this library in an entirely different direction to Helia and libp2p. Then there's all the folks outside of PL doing weird and whacky things! But my point here is that yes, I would very much love to have maintenance help in this library but the thing I would really like to protect in all of this is the whole reason js-multiformats exists—to avoid bloat in everyone's dependency chains. Because this library sits within everyone's libraries and applications when they touch anything IPFS/IPLD/libp2p related (even remotely related!), it has to be lean, both on disk and within the import trees (for bundle sizes). Even adding seemingly trivial things like sha1 are a concern for this reason. That's why this library exists and why it looks like it does. So, I'm very hesitant to just hand this off to the js-libp2p team in whole, but I'd very much like help in continuing to maintain it because I'm really not the best person to deal with the constant onslaught of people asking why it doesn't work for their very specific bundling or building toolchain (because that's the majority of issues we get for our simpler libraries!). That's also the lens I use for viewing this TypeScript question—does it help with any of that? I'm dubious, but open to being convinced because it seems a lot of the issues get filed by people complaining about typedefs (but it's not clear to me that continuing to fix what we have is not better than converting the entire thing). |
Exactly 💯, multiformats is literally every where and is used by bluesky, walletconnect, fission, ceramic and many many others (everyone using uint8arrays by @achingbrain 😁). Scroll through https://www.npmjs.com/browse/depended/multiformats to see direct dependents. I available to help @rvagg maintain this repo. |
I would really encourage to focus conversation around the problems been solved / introduced, inconveniences been solved / introduced and restraining from other sentiments and speculations like ES syntax extensions that JS engines will implement and ship. I have no doubt you are all well meaning, but I can't help but suspect bias. When it is true and JS engines adopt TS syntax, arguments voiced against TS syntax adoption will no longer be valid making transition a win, which I would whole heartedly support. Until such time, plans that JS engines may have do not address any arguments voiced against it. So please lets focus on facts today when facts change we can revisit decisions that were based on them. So far I'm hearing following arguments (please correct me if I have left something out):
If we have a complete list of problems / inconveniences we could use empirical method, comparing tradeoffs to come to a decision. I also should mention that contribution list only accounts for commits, which fails to account for reviews, that I have personally been involved in and have intention to continue to do so (regardless of decision here). Worth mentioning that rest of the contributors (excluding @juliangruber) include @RangerMauve and other web3.storage members, who I suspect fall into the jsdoc side of the argument. |
Lots of conversation here and I do want to get a decision made. @wemeetagain : are there other arguments you think should be added to @Gozala 's list? @wemeetagain : not critical here, but can you link to how you generated the consumer results above? @Gozala :
Below is a more exhaustive list of contributors for different actions over the last 12 months:
|
A quick update: discussions on the next steps will stall for a week or two as js-libp2p maintainers and IPLD maintainers are out of office |
Hey @p-shahi {
"compilerOptions": {
"target": "es2016",
"module": "commonjs",
} |
100%, it's less maintenance, and there is much less chance for your interface types to mismatch your source code.
ts-node makes this a non-issue. You can REPL ts-code just as well, and many people in the TS ecosystem do, if they don't just use deno or bun.
This is likely due to build configuration or minification problems. source maps will give you pure JS to look at in the browser, and I've rarely had issues here. One problem I've run into that can be bypassed with some minor setup is workspace files in chromium editors, where you can change the code directly in your devtools. However, this is likely only relevant for front-end devs and mitigated easily with live-reload.
I completely disagree here. Having migrated a number of projects to Typescript, I have to call out that the time you spend in that pain will lessen significantly, whereas the disconnect between JSDoc types and your actual code (regardless of ts-check pragma) will cause much more pain.
Are we talking stacktraces from a binary in a terminal, browser stacktraces, or reported stacktraces in an analytics/metrics dashboard/tool? I have run into this as well, but depending on where you're viewing the stacktraces and what source-maps you have, this is a one-time fix, or at most a minor inconvenience because more of your errors are caught during compile-time with typescript.
Sure, but the solution you provide is you writing typescript, and you can have the original source be typescript and still write your interfaces in a separate TS file.
You write typescript, to solve the problems JSDoc has.
When you start using more generics and higher-kinded types, you will run into many problems, and end up writing typescript. No matter how painful the disconnect between JSDoc and source can get (or any other concerns here), my main issue with JSDoc types is Generics. And with this library, better generics are an absolute must.
We need generics; more & better generics because of exactly where this library sits in the ecosystem. And once we have that, we can improve typings in consuming libraries by enhancing our types with better conditional types. I think there have been a lot of discussion around how JSDoc can work and currently is, but I haven't seen any challenges to whether typescript is better for ensuring better type-safety in the source code. So it really seems like the main problems are:
and for 2, many of the workarounds/solutions @Gozala is already using involve directly writing typescript anyway. |
The problem is that it's all of the above and more. Again as you pointed out you can address most if you put enough effort into them, however my point is precisely that it requires putting some effort every time which is a tradeoff.
I am not against typescript, I'm just saying you can apply it where it actually provides value as opposed to just rewriting everything in TS because all the other code is TS. I tend to apply TS syntax for code that does not have runtime representation to take advantage of type checking.
There is a implied assumption here. Perhaps it would be a more productive direction to explore what better generics would look like and what problems would they be solving. Don't get me wrong I think they can be extremely useful along with phantom types, yet in my experience overusing them can introduce unnecessary complexity (I suppose dI'm siding with Elm camp here, but that is irrelevant). Naturally if there are some improvements that we can not make with current setup will be a good case for changing it. |
There are improvements that we cannot make with the current setup. It was only after rewriting this library in typescript (#251) that some refactoring opportunities became visible to me. I think we would like to further simplify this library, if possible, but due to vanilla javascript coding paradigms, verbosity of jsdocs, lack of confidence of correctness of large code changes, larger refactoring of the codebase is sadly out of the question in its current state. Typescript provides value by simplifying maintenance burden.
|
@wemeetagain Action items discussed yesterday
|
Following up on the comment above... js-libp2p and Helia maintainers met during a colo. We understand this isn't a clear-cut answer and that this increases some friction for some contributors/users. There is a weight of opinion across the Helia and js-libp2p teams who contribute and use this library to move this TS migration forward. We're planning to make this happen by 2023-10-03 unless another maintainer says "no" sooner. Before merging #251, the plan is to:
That said, this issue has also highlighted ownership ambiguities. I have created a separate issue to handle this (#273 ). |
This is great @BigLep !! Finally a decision, that removes friction! |
Fixes #249 - update aegir to get latest linter rules - minimal changes to vendored code - eslint-disable files, add inferred types as jsdocs, move vendor directory under src - convert all .js files to .ts - apply all jsdoc type annotations as typescript type annotations - fix all resulting linter and type issues - Special attention given to keep any helpful jsdocs --------- Co-authored-by: Alex Potsides <[email protected]>
Fixes #249 - update aegir to get latest linter rules - minimal changes to vendored code - eslint-disable files, add inferred types as jsdocs, move vendor directory under src - convert all .js files to .ts - apply all jsdoc type annotations as typescript type annotations - fix all resulting linter and type issues - Special attention given to keep any helpful jsdocs BREAKING CHANGE: this module is now TypeScript - the API has not changed but releasing as a major for saftey --------- Co-authored-by: Alex Potsides <[email protected]>
## [13.0.0](v12.1.3...v13.0.0) (2023-12-20) ### ⚠ BREAKING CHANGES * this module is now TypeScript - the API has not changed but releasing as a major for saftey ### Features * convert codebase to typescript ([#251](#251)) ([fcee86e](fcee86e)), closes [#249](#249) ### Trivial Changes * **deps:** bump actions/setup-node from 3.8.2 to 4.0.0 ([139b3c2](139b3c2))
## [13.0.0](v12.1.3...v13.0.0) (2023-12-20) ### ⚠ BREAKING CHANGES * this module is now TypeScript - the API has not changed but releasing as a major for saftey ### Features * convert codebase to typescript ([#251](#251)) ([fcee86e](fcee86e)), closes [#249](#249) ### Trivial Changes * **deps:** bump actions/setup-node from 3.8.2 to 4.0.0 ([139b3c2](139b3c2))
Fixes #249 - update aegir to get latest linter rules - minimal changes to vendored code - eslint-disable files, add inferred types as jsdocs, move vendor directory under src - convert all .js files to .ts - apply all jsdoc type annotations as typescript type annotations - fix all resulting linter and type issues - Special attention given to keep any helpful jsdocs BREAKING CHANGE: this module is now TypeScript - the API has not changed but releasing as a major for saftey --------- Co-authored-by: Alex Potsides <[email protected]>
## [13.0.0](v12.1.3...v13.0.0) (2023-12-20) ### ⚠ BREAKING CHANGES * this module is now TypeScript - the API has not changed but releasing as a major for saftey ### Features * convert codebase to typescript ([#251](#251)) ([56bbb96](56bbb96)), closes [#249](#249) ### Trivial Changes * **deps:** bump actions/setup-node from 3.8.2 to 4.0.0 ([139b3c2](139b3c2))
Yahoo! |
Considering lot of JS IPFS codebase is in TypeScript, when do you expect multiformats to be converted to TypeScript? What are the blockers?
The text was updated successfully, but these errors were encountered: