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

chore: use Deno 2.1 in tests #15183

Merged
merged 11 commits into from
Jan 16, 2025
Merged

chore: use Deno 2.1 in tests #15183

merged 11 commits into from
Jan 16, 2025

Conversation

vkarpov15
Copy link
Collaborator

Summary

We're using an out-of-date version of Deno, should test against latest

Examples

Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, though it seems deno now requires "implicit js" files to have package.json "type": "commonjs" as seen in the failing tests:

     info: Deno supports CommonJS modules in .cjs files, or when the closest
          package.json has a "type": "commonjs" option.
    hint: Rewrite this module to ESM,
          or change the file extension to .cjs,
          or add package.json next to the file with "type": "commonjs" option,
          or pass --unstable-detect-cjs flag to detect CommonJS when loading.
    docs: https://docs.deno.com/go/commonjs

This should be a non-breaking change as (at least in node) a absense of the type field should imply commonjs.

@vkarpov15
Copy link
Collaborator Author

We should be safe to add "type": "commonjs" because of Node docs here:

image

In the interest of caution, I'll move this back to 8.10.

@vkarpov15 vkarpov15 added this to the 8.10 milestone Jan 15, 2025
test/deno.js Show resolved Hide resolved
@vkarpov15 vkarpov15 changed the base branch from master to 8.10 January 16, 2025 21:08
@vkarpov15 vkarpov15 merged commit 5a056c3 into 8.10 Jan 16, 2025
79 checks passed
@hasezoey hasezoey deleted the vkarpov15/deno-upgrade branch January 17, 2025 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants