-
Notifications
You must be signed in to change notification settings - Fork 825
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
chore(deps): Upgrade to Typescript 5 #4323
Conversation
@jcarvalho seems the CI is not working. I seem to remember this is from kubecon contribfest right? Are you still working on this or should someone else pick it up? |
This came from the Contribfest, yes, but haven't yet had the chance to look at the CI errors, will look into it during the weekend. |
@dyladan The fix was actually quite simple (updated initially to TS 5.2, but then TS 5.3 came out and my cached |
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## next #4323 +/- ##
==========================================
- Coverage 92.24% 90.32% -1.92%
==========================================
Files 332 161 -171
Lines 9437 3783 -5654
Branches 1999 841 -1158
==========================================
- Hits 8705 3417 -5288
+ Misses 732 366 -366 |
The EoL Node Tests failing are to be expected, as Typescript 5 requires Node 12, and the earliest version of Typedoc that supports TS 5.2 or higher requires Node 16 (although it seems the Node 14 tests are still passing?). I seems that if we stick to Typescript 5.1 we should be able to stay in a version of Typedoc that still supports Node 12. What's the earliest version of Node you're targeting for the |
We had hoped to bump to latest, but if it messes with API EOL testing we'll have to take a look. It might be possible to install old versions of typescript for just those tests. |
Ah, looks like it's not related to Typedoc at all. Typescript 5.0 bumped the minimum Node version to 12, and 5.1 bumped it to 14 Link (which explains the cryptic error we're seeing now).
Do you mean using this? If we do so, we might be limiting ourselves to not using Typescript features from 5.1 and above, and in that case it might just be better to stick to the Typescript 5.0 line (and require Node 12 or higher). |
Why would that limit the typescript features we can use? The EOL tests don't affect anything outside of the API module. Also, even later versions of typescript can still have their output run by old node versions, the compiler just doesn't run on those versions. A user with node 8 not using typescript could definitely use code emitted by the typescript 5.2 compiler. |
Based on Typescript 5.1's Release Notes, it seems that Node 14.17 is a minimum Runtime Requirement, as that version of Typescript emits Javascript that requires Node 14 or higher, so even if we compiled on a newer version of Node, the Javascript output by the compilation process wouldn't run on the older Node versions. |
Ah interesting. That should be OK everywhere except the API. |
Given even 5.0 requires a Node 12 or higher at runtime, and we're still supporting Node 8 and 10, should we just skip the Typescript 5 upgrade for the API package (leaving it with TS 4), and upgrade everything else to the latest TS 5.3? |
I have a different understanding. This is the requirement to run typescript. The emitted code depends on the typescript is a dev dependency. So even if we generated code which runs on node 8 it doesn't require to run typescript using node 8. There is no need to build and run with the same version. But typescript might emit .d.ts files which are not understood by older typescript versions. Therefore the requirement to build with recent versions leaks into user setups. I don't think sticking on older typescript is a good choice even if it is only the API package. It's just a matter of time that we end up in compatibility issues because of this because some other tool in use expects more up to date peers. |
This was also my understanding.
Since typescript introduces these changes sometimes in minor releases, we can really only guarantee our output builds with the version we use (or later). Because this introduces a build-time node version dependency on our users, it is effectively a minimum node version. It may be possible to build with a later version and still run on the old node version but in practice most won't do this and I wouldn't want to encourage it. We're going to have to drop support for node 8 in the API eventually, and if we're not going to rev the major version, we're going to have to do it in a minor version at some point. Today may not be that day, but we should probably consider how we want to go about that change. |
This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days. |
This PR was closed because it has been stale for 14 days with no activity. |
Which problem is this PR solving?
It upgrades to the latest version of Typescript.
Fixes #3955
Short description of the changes
Upgrades Typescript and Typedoc (as the version in use wasn't compatible with TS 5).
Type of change
How Has This Been Tested?
Checklist: