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

Can you update your Deno version? #10809

Closed
lucacasonato opened this issue Sep 16, 2024 · 18 comments
Closed

Can you update your Deno version? #10809

lucacasonato opened this issue Sep 16, 2024 · 18 comments
Assignees
Labels
deno Issues related to deno enhancement New feature or request
Milestone

Comments

@lucacasonato
Copy link

Hey folks! I'm one of the Deno maintainers. We get issues like these denoland/deno#25654 every couple of days from your users. The underlying cause of this panic has been resolved for a couple of Deno releases. However, because you bundle with Deno 1.41.1, none of your users have gotten this fix yet. Could you release an update of quarto with a newer Deno version, like 1.46.3? That way users will stop running into this panic :)

If you haven't upgraded due to an issue in Deno, please let me know - if we know what you are running into we can prioritize it!

@mcanouil
Copy link
Collaborator

Thanks for reaching out.
Indeed, there are quite few issues with Deno (https://github.com/quarto-dev/quarto-cli/issues?q=is%3Aissue+is%3Aopen+label%3Adeno).
@cscheid will likely take on your offer and get back to you on those issues.

@mcanouil mcanouil added the triaged-to Issues that were not self-assigned, signals that an issue was assigned to someone. label Sep 16, 2024
@lucacasonato
Copy link
Author

It seems that #7292, #9466, and #10130 would all be resolved by just upgrading (they are the same root certificate panic that we have now fixed).

@cscheid
Copy link
Collaborator

cscheid commented Sep 16, 2024

@lucacasonato I'm so sorry that our users are reporting this to you! That's really not good form on our part. We definitely want to update Deno for 1.6 and we're very behind on that (exactly because of the stream issue you unblocked us!)

I'll also try to change 1.6 so that error messages look more like "please please file this on quarto-cli before deno" so there's fewer future annoyance from our side onto yours.

@cscheid cscheid added this to the v1.6 milestone Sep 16, 2024
@lucacasonato
Copy link
Author

Yeah don't worry about the issues on our repo. We can deal with them - the issues here came from a specific callout from us to report the error in a panic, so you don't really have much you could change on the output anyway (example denoland/deno#25713).

@cscheid
Copy link
Collaborator

cscheid commented Sep 18, 2024

@lucacasonato I'm currently blocked on updating Deno to 1.46.3 with two deno vendor issues. I posted them on the Deno discord help (I don't know if the URLs I get from discord transfer across users, but if they do: Question 1, Question 2). I know this is asking a lot, but is there anyone in the Deno team who's a deno vendor expert that could help me sort out our import map so we can move from deno.land/std to jsr/std? Thanks!

@dsherret
Copy link

@cscheid those links don't work for me for some reason. Can you post the issues here?

@cscheid
Copy link
Collaborator

cscheid commented Sep 23, 2024

@dsherret Of course (and thank you!). I'd also be happy to take this offline or on a more appropriate forum.

Issue 1

In Deno 1.46.3, it seems that deno vendor doesn't like to see <reference types=''> entries in source code. Concretely, I can't seem to call deno vendor in any project that eventually sees https://deno.land/[email protected]/crypto/_wasm/lib/deno_std_wasm_crypto.generated.mjs?source=#L5, since that causes the following error:

error: Expected a JavaScript or TypeScript module, but identified a Unknown module. Importing these types of modules is currently not supported.
  Specifier: https://deno.land/[email protected]/crypto/_wasm/lib/deno_std_wasm_crypto.generated.d.mts
    at https://deno.land/[email protected]/crypto/_wasm/lib/deno_std_wasm_crypto.generated.mjs:5:22

Is that a bug?

Issue 2

Hi - I'm hoping to get some guidance of best practices of migrating a (n admittedly complicated) import_map.json file from using https://deno.land/[email protected] to jsr

Our import map at Quarto currently contains entries like this:

    "archive/": "https://deno.land/[email protected]/archive/",

We use this syntax because it allows us to do imports in our TS code like this:

import { Tar } from "archive/tar.ts";
import { Zip } from "archive/zip.ts";, etc.

This allows our import map to not worry about which of the tar.ts, zip.ts are going to be ultimately imported in our code.

My specific problem is I'm having a hard time converting that to a jsr import, say

    "archive/": "jsr:@std/archive@^0.224.0/",

If I maintain the trailing / in the syntax, I get a failure like this when calling deno vendor:

error: Failed to resolve the specifier ""archive/tar.ts"" as its after-prefix
            portion ""tar.ts"" could not be URL-parsed relative to the URL prefix
            "jsr:@std/archive@^0.224.0/" mapped to by the prefix "archive/"

But if I switch to an import_map statement like

    "archive": "jsr:@std/archive@^0.224.0",

then it seems that I need to declare each of the imports (tar, zip, etc) separately in the import map itself. That would be a big bummer and a source of ongoing maintenance toil for us, because the import_map will become much more brittle than before. Is there a way to import from jsr and retain the flexibility with paths we used to have in import maps and deno vendor? Thanks for the help!

@dsherret
Copy link

For issue 1, do you have a branch I can try this out on?

For issue 2, change "archive/": "jsr:@std/archive@^0.224.0/", to "archive/": "jsr:/@std/archive@^0.224.0/", (this is necessary in an import map, but not a deno.json because jsr:@std... is not a base url unfortunately... if we were doing this again we'd require the / at the start)

@cscheid
Copy link
Collaborator

cscheid commented Sep 23, 2024

For issue 1, do you have a branch I can try this out on?

Try this:

$ git clone [email protected]:quarto-dev/quarto-cli
...
$ cd quarto-cli
$ git checkout chore/deno-update-2024-09
...
$ ./configure.sh
...
$ ./package/scripts/vendoring/vendor.sh
Revendoring quarto dependencies
/tmp/quarto-cli/src /tmp/quarto-cli
⚠️ Warning: `deno vendor` is deprecated and will be removed in Deno 2.0.
Add `"vendor": true` to your `deno.json` or use the `--vendor` flag instead.
error: Expected a JavaScript or TypeScript module, but identified a Unknown module. Importing these types of modules is currently not supported.
  Specifier: https://deno.land/[email protected]/crypto/_wasm/lib/deno_std_wasm_crypto.generated.d.mts
    at https://deno.land/[email protected]/crypto/_wasm/lib/deno_std_wasm_crypto.generated.mjs:5:22
deno vendor failed (likely because of a download error). Please run the configure script again.

@cscheid
Copy link
Collaborator

cscheid commented Sep 23, 2024

For issue 2, change "archive/": "jsr:@std/archive@^0.224.0/", to "archive/": "jsr:/@std/archive@^0.224.0/", (this is necessary in an import map, but not a deno.json because jsr:@std... is not a base url unfortunately... if we were doing this again we'd require the / at the start)

This worked. Thank you! That's very encouraging.

Plenty of fixing for me to do before I can report further, but we might be unblocked through 1.46.3 assuming this works out. I'll get back to you.

@cscheid
Copy link
Collaborator

cscheid commented Sep 23, 2024

I'm going to use this issue to track other blocking problems for Quarto.

@cscheid
Copy link
Collaborator

cscheid commented Sep 23, 2024

We're almost there. I'll wait a couple of days for a response on the std/yaml export that disappeared before just pulling in js-yaml ourselves as an external dependency. The response from std/yaml doesn't give me confidence that Deno's yaml library will restore the js-yaml features, so I went ahead and removed our dependence on it.

@cscheid cscheid removed the triaged-to Issues that were not self-assigned, signals that an issue was assigned to someone. label Sep 23, 2024
@cscheid
Copy link
Collaborator

cscheid commented Sep 24, 2024

For issue 2, change "archive/": "jsr:@std/archive@^0.224.0/", to "archive/": "jsr:/@std/archive@^0.224.0/", (this is necessary in an import map, but not a deno.json because jsr:@std... is not a base url unfortunately... if we were doing this again we'd require the / at the start)

@dsherret Is this syntax expected to work for non-@std applications? I find myself being unable to import Cliffy from JSR by any means.

Our source code contains entries like

import { Command } from "cliffy/command".

Here's what I'm trying to do and failing, with any syntax:

1

    "cliffy/": "jsr:/@[email protected]/",

This fails with

JSR package not found: @[email protected]/command

2

    "cliffy/command": "jsr:/@cliffy/[email protected]",

This fails with

error: Version tag not supported in jsr specifiers.

3

    "cliffy/command": "jsr:/@[email protected]/command",

fails with

error: JSR package not found: @[email protected]/command

(And in case you're curious, Cliffy from deno.land is broken under import maps because it doesn't actually type check: c4spar/deno-cliffy#744)

@dsherret
Copy link

dsherret commented Sep 24, 2024

The package name is @cliffy/command (https://jsr.io/@cliffy/[email protected]) so the version constraint will come after that:

"cliffy/command": "jsr:/@cliffy/[email protected]"

Also, remove the v

@cscheid
Copy link
Collaborator

cscheid commented Sep 24, 2024

Also, remove the v

Ah, I see. (It did have a v in the deno.land version, confusingly: https://deno.land/x/[email protected])

@cscheid
Copy link
Collaborator

cscheid commented Sep 24, 2024

It seems we're blocked on Cliffy - with the new Deno version, rc.3, rc.4 and rc.5 all fail to typecheck with a simple repro like so:

$ cat main2.ts
import { Checkbox } from "jsr:@cliffy/[email protected]"
$ deno --version
deno 1.46.3 (stable, release, aarch64-apple-darwin)
v8 12.9.202.5-rusty
typescript 5.5.2
$ deno vendor main2.ts
⚠️ Warning: `deno vendor` is deprecated and will be removed in Deno 2.0.
Add `"vendor": true` to your `deno.json` or use the `--vendor` flag instead.
Vendored 111 modules into vendor/ directory.

To use vendored modules, specify the `--import-map vendor/import_map.json` flag when invoking Deno subcommands or add an `"importMap": "<path_to_vendored_import_map>"` entry to a deno.json file.
$ deno check --all main2.ts --import-map vendor/import_map.json
Check file:///Users/cscheid/Desktop/daily-log/2024/09/24/cliffy/main2.ts
error: TS2345 [ERROR]: Argument of type '(a: MatchedOption<TValue, TOption, TGroup>, b: MatchedOption<TValue, TOption, TGroup>) => number' is not assignable to parameter of type '(a: MatchedOption<unknown, GenericListOptionSettings<unknown>, GenericListOptionGroupSettings<unknown, GenericListOptionSettings<unknown>>>, b: MatchedOption<...>) => number'.
  Types of parameters 'a' and 'a' are incompatible.
    Type 'MatchedOption<unknown, GenericListOptionSettings<unknown>, GenericListOptionGroupSettings<unknown, GenericListOptionSettings<unknown>>>' is not assignable to type 'MatchedOption<TValue, TOption, TGroup>'.
      Type 'GenericListOptionSettings<unknown>' is not assignable to type 'TOption'.
        'TOption' could be instantiated with an arbitrary type which could be unrelated to 'GenericListOptionSettings<unknown>'.
        .sort(sortByDistance);
              ~~~~~~~~~~~~~~
    at file:///Users/cscheid/Desktop/daily-log/2024/09/24/cliffy/vendor/jsr.io/@cliffy/prompt/1.0.0-rc.5/_generic_list.ts:796:15

TS2322 [ERROR]: Type 'MatchedOption<unknown, GenericListOptionSettings<unknown>, GenericListOptionGroupSettings<unknown, GenericListOptionSettings<unknown>>>[]' is not assignable to type 'MatchedOption<TValue, TOption, TGroup>[]'.
  Type 'MatchedOption<unknown, GenericListOptionSettings<unknown>, GenericListOptionGroupSettings<unknown, GenericListOptionSettings<unknown>>>' is not assignable to type 'MatchedOption<TValue, TOption, TGroup>'.
          children,
          ~~~~~~~~
    at file:///Users/cscheid/Desktop/daily-log/2024/09/24/cliffy/vendor/jsr.io/@cliffy/prompt/1.0.0-rc.5/_generic_list.ts:802:11

    The expected type comes from property 'children' which is declared here on type 'MatchedOption<TValue, TOption, TGroup>'
      children: Array<MatchedOption<TValue, TOption, TGroup>>;
      ~~~~~~~~
        at file:///Users/cscheid/Desktop/daily-log/2024/09/24/cliffy/vendor/jsr.io/@cliffy/prompt/1.0.0-rc.5/_generic_list.ts:157:3

Found 2 errors.

Ideally we'd keep rc.3, since between rc.3 and rc.5, Cliffy changed its interface in a breaking manner, removing a number of exported classes that we use in Quarto.

As a reminder, we really do need deno vendor because we crucially depend on deno bundle to ship our distribution, and we haven't found a way out of that deadlock yet (and the move to the JSR import specifiers just made the issue potentially worse).

@dsherret @lucacasonato If you can help out here with any ideas, that would be deeply appreciated, since we're now blocked. I'd be happy knowing how to disable type checking for only the Cliffy library, for example.

@cscheid
Copy link
Collaborator

cscheid commented Sep 25, 2024

We'll drop Cliffy in 1.7: #10878

@cscheid
Copy link
Collaborator

cscheid commented Oct 2, 2024

1.6.19 runs Deno 1.46.3 now.

@cscheid cscheid closed this as completed Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deno Issues related to deno enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants