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

Support --importmap with deno bundle #4379

Closed
vtenfys opened this issue Mar 15, 2020 · 4 comments · Fixed by #4651
Closed

Support --importmap with deno bundle #4379

vtenfys opened this issue Mar 15, 2020 · 4 comments · Fixed by #4651
Assignees

Comments

@vtenfys
Copy link

vtenfys commented Mar 15, 2020

Reproduction steps

  1. Create a script with some imports, e.g. index.js
  2. Create an import map to resolve these imports, e.g. import-map.json
  3. Run deno bundle --importmap=import-map.json index.js

Example where this can be reproduced: https://github.com/davidbailey00/deno-npm-demo

Expected result

It works, like with deno --importmap=import-map.json index.js

Actual result

The following output is displayed:

error: Found argument '--importmap' which wasn't expected, or isn't valid in this context

USAGE:
    deno bundle [OPTIONS] <source_file> [out_file]

For more information try --help

Additional notes

I would love to create a PR to support this, but might need some pointing in the right direction since I'm relatively new to Rust programming! Would it be a matter of adding .arg(importmap_arg()) to the bundle_subcommand function?

Based on #2687 I would expect this to already work, though it looks the behaviour has changed since this PR?

@kitsonk
Copy link
Contributor

kitsonk commented Mar 15, 2020

I suspect so. There really shouldn't be much more than accepting it just like the run subcommand. The rest of the infrastructure should "just work".

@vtenfys
Copy link
Author

vtenfys commented Mar 15, 2020

diff --git a/cli/flags.rs b/cli/flags.rs
index 9e1fbf5d..b954ea73 100644
--- a/cli/flags.rs
+++ b/cli/flags.rs
@@ -618,6 +618,7 @@ To change installation directory use -d/--dir flag:
 
 fn bundle_subcommand<'a, 'b>() -> App<'a, 'b> {
   SubCommand::with_name("bundle")
+    .arg(importmap_arg())
     .arg(
       Arg::with_name("source_file")
         .takes_value(true)

I tried that and got the following error instead:

$ ~/Code/Rust/deno/target/debug/deno bundle --importmap=web_modules/import-map.json demos/preact.js
error: Uncaught URIError: relative import path "htm/preact" not prefixed with / or ./ or ../ Imported from "file:///Users/david/Code/JavaScript/deno-npm-demo/demos/preact.js"
► $deno$/ops/dispatch_json.ts:43:11
    at unwrapResponse ($deno$/ops/dispatch_json.ts:43:11)
    at sendSync ($deno$/ops/dispatch_json.ts:72:10)
    at resolveModules ($deno$/ops/compiler.ts:11:10)
    at resolveModules ($deno$/compiler/imports.ts:63:22)
    at processImports ($deno$/compiler/imports.ts:147:27)
    at processImports ($deno$/compiler/imports.ts:156:13)

It seems that the argument is parsed correctly but the bundle subcommand doesn't make use of it 🤔

@kitsonk
Copy link
Contributor

kitsonk commented Mar 15, 2020

I don't have everything in my head at the moment to give you strong guidence, but if it were me, I would compare:

$ deno -L debug bundle --importmap=web_modules/import-map.json demos/preact.js

With:

$ deno -L debug run --importmap=web_modules/import-map.json demos/preact.js

The TypeScript cli/js/compiler.ts resolves modules in exactly the same way, so it is likely something is not being set in cli/file_fetcher.rs but I don't know the whole import maps logic that well. IIRC @kevinkassimo had a hand in the implementation.

@0x1af2aec8f957
Copy link

I expect to support -- importmap during installation。

example:

deno install --unstable --importmap=import_map.json test test.ts

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 a pull request may close this issue.

4 participants