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

Deno2 updates #90

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Deno2 updates #90

wants to merge 5 commits into from

Conversation

jimmont
Copy link

@jimmont jimmont commented Nov 18, 2024

first/preliminary pass at update related to deno v2, fixing #91

  • bundle script and task added
  • test of distributable and task for it added
  • global for window updated
  • jsr imports for latest versions updated, with comments for as it was unclear why previous attempt was reverted related to this and what might be done about those things, whatever they might be
  • tests pass

…or distributable; use jsr with comments for possible other environments aiming to use current versions in deno 2, with testing working; though it's unclear why prior attempts were reverted;
@emrahcom
Copy link
Contributor

Is window or globalThis needed for crypto.subtle.generateKey?
It should work without them.

@timonson
Copy link
Member

Is window or globalThis needed for crypto.subtle.generateKey? It should work without them.

Using crypto.subtle.generateKey directly can lead to issues if crypto or subtle has been shadowed in the local scope, causing unexpected failures. Opting for globalThis.crypto.subtle.generateKey ensures explicit clarity and better compatibility across diverse environments, avoiding confusion in larger codebases or contexts where crypto isn't globally available.

@timonson
Copy link
Member

Looks good to me. Can you tell me if I have to configure something on the jsr website for that?

@jimmont
Copy link
Author

jimmont commented Nov 20, 2024

@timonson I added the fields for publishing to jsr per https://jsr.io/docs/publishing-packages and deno publish --dry-run looks ok. Please check the version and exports fields in the last change to deno.json.

If possible I'd prefer to leave the publish and version change to you or whomever wants to publish this new version, so feel free to make those changes when ready and without my help--the field in deno.json is simply "version". I guessed a major version bump simply because I'm unsure how backward compatible this is for deno v1, etc.

Hope this helps.

It's also worth noting that if you have hesitations related to using jsr it's also possible to publish this here via github using a raw.githubusercontent.com or gh-pages url. I think this might be the most flexible option if browser support is important. I haven't looked closely into the pros and cons of using jsr.

@jimmont
Copy link
Author

jimmont commented Nov 21, 2024

my brief checks of the ci failures suggests deno upgrade is needed, denoland/fresh#2622

I'm beginning to think publishing to jsr may not be in line with the compatibility goals here, and simply publishing here on gh, or similar, and on deno.land/x/. More elaboration in the jsr-feedback deno discord here (2024-11-21) https://discord.com/channels/684898665143206084/1203185670508515399/1308971117968621589

fwiw I'd recommend closing this PR and sticking with deno.land and perhaps also http://raw.githubusercontent.com or gh-pages if a live demo in the client is wanted. Hope this useful. If I can help more just let me know.

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.

3 participants