denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
98.12k stars 5.4k forks source link

Deno gfm vendoring issues #21415

Open mcandeia opened 12 months ago

mcandeia commented 12 months ago

Version: Deno 1.38.4

I encountered a bug related to vendoring dependencies while using deno. The problem becomes apparent when attempting to vendor the deno-gfm library. For detailed information on how to reproduce this issue, please refer to the repository:

https://github.com/mcandeia/deno-vendor-bug

To reproduce, fork the repository and try to run with --no-remote options poiting to vendor/import_map.json file

dsherret commented 12 months ago

Does it work with the new vendor feature (should be much more reliable)? You might want to use that instead https://deno.com/blog/v1.37#vendor-as-cache-override-unstable (though it doesn't work on deploy atm)

Note that the deno vendor subcommand may go away in the future and be moved to a separate library: https://github.com/denoland/deno/issues/20584

mcandeia commented 12 months ago

@dsherret I couldn't make it work either, it's not clear to me if I should run deno cache --vendor main.tsx and then I can run with --no-remote ? previously It was possible by using the generated import_map inside vendor folder, but looks like it was removed, right?

mcandeia commented 12 months ago

I tried with --vendor using deno run --vendor main.tsx and it seemed to work, but with a large project I have to remove node_modules/.deno to work, probably something that changes based on the SO/VM.

Also: I noticed that the node_modules still being downloaded even with --vendor, is that correct?

image image
mcandeia commented 11 months ago

ping @dsherret

dsherret commented 11 months ago

I couldn't make it work either, it's not clear to me if I should run deno cache --vendor main.tsx and then I can run with --no-remote?

Basically once "vendor": true is in the config file, then you can run deno cache main.tsx and then from then use deno run with --no-remote. If not setting in the config file, then you need to provide --vendor to each command.

Also: I noticed that the node_modules still being downloaded even with --vendor, is that correct?

They should be vendored on first run.

but with a large project I have to remove node_modules/.deno to work, probably something that changes based on the SO/VM.

You can disable the node_modules auto-install/vendoring by setting "nodeModulesDir": false.


So I looked into this issue more and I think this is a design limitation with import maps. I'm not super knowledgable on them, but it seems like there's no way to map a specifier like https://esm.sh/marked@5.1.0/ to something like ./esm.sh/marked@5.1.0.js. When I try I get Invalid target address "file:///V:/deno-vendor-bug/vendor/esm.sh/marked@5.1.0.js" for package specifier "https://esm.sh/marked@5.1.0/". Package address targets must end with "/".

I opened a PR https://github.com/denoland/deno-gfm/pull/83 -- that should fix this and not require code changes.

dsherret commented 11 months ago

Upgrading to deno-gfm 0.3.0 should fix the issue now.

mcandeia commented 11 months ago

Thanks, will check @dsherret

mcandeia commented 11 months ago

Now it's a different problem, Tested with: https://github.com/deco-sites/start

(using Deno 1.39.1) deno cache --vendor dev.ts will generate a vendor folder with no import_map generated. I don't know if I did something wrong, but couldn't make it work @dsherret

image
dsherret commented 11 months ago

Yeah, that's expected. The --vendor feature (recommended use as { "vendor": true } in deno.json) is a completely different more reliable way of doing vendoring that does a cache override and doesn't use an import map (https://deno.com/blog/v1.37#vendor-as-cache-override-unstable). I wouldn't recommend using it yet though because it's not supported on Deploy yet.

Regarding the original deno vendor issue, it seems there's a new issue that's surfaced:

> deno vendor main.tsx
error: Failed resolving types. Relative import path "Instance" not prefixed with / or ./ or ../ and not in import map from "https://esm.sh/v135/marked@9.1.1/lib/marked.d.ts"
    at https://esm.sh/v135/marked@9.1.1/lib/marked.d.ts:553:39
mcandeia commented 11 months ago

Yeah, that's expected. The --vendor feature (recommended use as { "vendor": true } in deno.json) is a completely different more reliable way of doing vendoring that does a cache override and doesn't use an import map

Oh, I forgot to mention that the problem is exactly that, it changed my deno.json file and points to an import_map.json that do not exists @dsherret, it's easy to reproduce, just try with the repo that I mentioned

mcandeia commented 11 months ago

Just to let you know. I changed my approach because I couldn't make it work with vendor command nor --vendor option. It was much simpler to just zip the entire DENO_DIR and share it between machines. It seems to be a poor-man's solution, but I think vendoring is not a deno priority right now.