-
Notifications
You must be signed in to change notification settings - Fork 656
chore: support up to Deno LTS and refine supported Deno versions logic #6670
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6670 +/- ##
==========================================
- Coverage 94.69% 94.17% -0.53%
==========================================
Files 562 560 -2
Lines 46651 41613 -5038
Branches 6570 6561 -9
==========================================
- Hits 44177 39189 -4988
+ Misses 2431 2375 -56
- Partials 43 49 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@@ -9,6 +9,7 @@ | |||
"./diff": "./diff.ts", | |||
"./format": "./format.ts", | |||
"./styles": "./styles.ts", | |||
"./types": "./types.ts" | |||
"./types": "./types.ts", | |||
"./support": "./support.ts" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While mostly used internally, the public endpoint here is created for use by other public APIs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like recommending other libs to use this endpoint? I'm not in favor of that policy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It isn't. Like the other endpoints in this package, it's for internal use only. Do you suggest another name?
internal/support.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea here is that when code or tests will automatically fail once the Deno LTS version is upgraded in CI. Then, we can go in and update the relevant code, dropping support for functionality that's outside of LTS.
I don't think we're ready to make this change for the same reason as #6624 (comment) |
I should correct and clarify my previous points. These changes are not breaking. They do not force someone to upgrade from Deno v1 to v2. Rather, these changes update the expected error types for some APIs within In other words, a person can upgrade their packages after this PR is applied and nothing will break. It'll only change their expected error types and nothing else if they upgrade to Deno 2. And updating their expected error types, if they even check them, is trivial: The other changes in this PR simply ensure that we can move forward in not having to support a version of Deno that's unsupported by the runtime itself. And the linting changes wouldn't take hold until Deno 2.2 is no longer LTS, on Nov 1 of this year. I don't see a reason why Deno and the Standard Library should have two separate approaches to Deno LTS and I feel like this change should happen sooner or later. Alternatively, if we do want to support Deno 1, we should specify that in the documentation and maybe provide a good reason why the Standard Library deviates from Deno's LTS. I guess we can keep this open as Bartek advised in the preceding PR. |
This PR stops CI in v1. The changes after this PR can randomly break Deno v1 and they will be forced to update at that time. The last Deno v1.x version was released only 8 months ago https://github.com/denoland/deno/releases/tag/v1.46.3 while Node still supports v20 which was released more than 2 years ago https://nodejs.org/en/about/previous-releases I feel it's too early to start breaking it intentionally. Also we (std maintainers/developers) are not so blocked by the requirement of v1.x support as far as I'm aware. I don't see much benefit of making this change. |
I'm against dropping support for Deno 1.x . There is no runtime API in Deno 2 where you would say that the std libs absolutely could not function without it. Most packages like |
This PR changes the codebase such that it supports up to the Deno LTS version. The new
@std/internal/support
endpoint also helps with ensuring functionality is supported in the Deno LTS version.Closes #6669