Refracto: update more code to ts
Description
Refracto to have more file in ts: it's allow to have type-check and it's "securise" our code
[!NOTE] There are still files written in javascript, either they are used in the
next.config.jsso they remain in js for the form but if SWC transpiles the ts. And there are also node scripts, but you'd need to have a loader or be in node 22. So for the moment I'd rather do nothing for those.
Validation
- All test should pass
- Website should display correctly (use Vercel preview)
Check List
- [x] I have read the Contributing Guidelines and made commit messages that follow the guideline.
- [x] I have run
npm run formatto ensure the code follows the style guide. - [x] I have run
npm run testto check if all tests are passing. - [x] I have run
npx turbo buildto check if the website builds without errors. - NA I've covered new added functionality with unit tests if necessary.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Updated (UTC) |
|---|---|---|---|
| nodejs-org | ✅ Ready (Inspect) | Visit Preview | Oct 26, 2024 3:49pm |
FYI many of these files are intentionally done in plain JS -- so that they do not need to be transpiled by TypeScript.
There is no virtual benefit (necessarily) for having them in TypeScript. What's the motivation behind this change?
For me, it's ‘important’ to have them in typescript, for example next.dynamic, which is responsible for the entire content of the site. Because typescript offers type-checking which allows us to check that our code is correct and is supposed to help with the review.
We shouldn't all write in typescript in ‘wow that's typescript’ mode but see it more as a tool to limit errors.
FYI many of these files are intentionally done in plain JS -- so that they do not need to be transpiled by TypeScript. There is no virtual benefit (necessarily) for having them in TypeScript. What's the motivation behind this change?
For me, it's ‘important’ to have them in typescript, for example
next.dynamic, which is responsible for the entire content of the site. Because typescript offers type-checking which allows us to check that our code is correct and is supposed to help with the review. We shouldn't all write in typescript in ‘wow that's typescript’ mode but see it more as a tool to limit errors.
Again, they should not be in TypeScript. There are reasons here, and I'm not going to approve a PR just because you feel that something needs to be refactored to TypeScript.
before closing, can we get opinion from other members of the team @nodejs/nodejs-website
I believe the main reasons I'm against TypeScript here are:
- I'm the sort of maintainer who prefers TypeScript on userland (Apps), whereas plain JavaScript within Libraries or Configuration files.
- Some of these files should be able to be used independently of TypeScript or anything behind Turbopack/Webpack; I don't see the need for TypeScript here. Otherwise, we need to introduce yet another build step.
- It is the same reason why many files within the i18n package are plain JavaScript; we would introduce more dependencies.
This change is not a QoL update (IMO). Are these JSDocs fine, or is anyone having issues with them? I feel that having continuous refactoring is negative, and the time spent here on transforming these files into TypeScript could have been well spent elsewhere.
I don't necessarily have strong feelings here, but I won't remove my -1 until someone provides me a solid, convincing reason why TypeScript here would be better for these files, if not just a matter of preference.
the time spent here on transforming these files into TypeScript could have been well spent elsewhere.
This is perhaps the best argument - we have a lot on our plate with finite time. Everything we do is an investment, and it's hard to see the dividends or losses right away. I will stop with the analogy.
Considering the lack of consensus, I think the proper course of action is to close. @AugustinMauroy are you okay with that?
I am ok with that 👌