deno_emit icon indicating copy to clipboard operation
deno_emit copied to clipboard

Migrating from `importMap`/`importMapPath` to `imports` ?

Open lowlighter opened this issue 2 years ago • 3 comments

Do you have any examples about migrating from importMap/importMapPath to imports ?

Consider the following

const {files} = await Deno.emit(`data:application/javascript;base64,${encode(typescript)}`, {
  bundle: "classic",
  importMapPath: "imports.json",
  importMap: {imports: {"@project/": "/my/custom/path/"}},
})

Tried the following with x/emit based on typing signature:

const {code} = await bundle(`data:application/javascript;base64,${encode(typescript)}`, {
  type: "classic",
  imports: {"@project/": ["/my/custom/path/"]},
})

But it results in the following:

 Caused by: RuntimeError: memory access out of bounds
          at dlmalloc::dlmalloc::Chunk::size::h21e6256dacef1d17 (https://deno.land/x/[email protected]/lib/deno_emit_bg.wasm:1:3790959)
          at __rdl_realloc (https://deno.land/x/[email protected]/lib/deno_emit_bg.wasm:1:1391601)
          at __rust_realloc (https://deno.land/x/[email protected]/lib/deno_emit_bg.wasm:1:3716800)
          at __wbindgen_realloc (https://deno.land/x/[email protected]/lib/deno_emit_bg.wasm:1:3586796)
          at passStringToWasm0 (https://deno.land/x/[email protected]/lib/deno_emit.generated.js:92:11)
          at bundle (https://deno.land/x/[email protected]/lib/deno_emit.generated.js:175:50)
          at bundle (https://deno.land/x/[email protected]/mod.ts:83:10)
          at async A.#deno (file:///workspaces/itsudeno3/core/components/connection.ts:117:30)
          at async A.deno (file:///workspaces/itsudeno3/core/components/connection.ts:76:28)
          at async Object.<anonymous> (file:///workspaces/itsudeno3/builtin/connection/local/mod_test.ts:23:35)

lowlighter avatar May 19 '22 03:05 lowlighter

I think there are issues with this and the implementation wasn't complete. #15 may have partially fixed it, but it needs tests for it and created as a more complete feature.

kitsonk avatar May 20 '22 04:05 kitsonk

The imports do not seem to be handled correctly in the code. On this line: https://github.com/denoland/deno_emit/blob/c6f061dd8f9bb0c2f2b0709a4e9aaf657f4169b4/mod.ts#L160

...the import map is passed as a JSON string as the third argument to jsBundle/bundle. But here: https://github.com/denoland/deno_emit/blob/c6f061dd8f9bb0c2f2b0709a4e9aaf657f4169b4/lib/emit.generated.js#L226

...the imports are supposed to be the fourth argument. I tried fixing this, and I got an error "invalid type: string, expected a map." So the import map is not supposed to be stringified. I fixed that. Now I am getting an error: "Error: relative URL without a base." All the URLs I am passing into bundle are absolute. Maybe it is something about relative imports.

dgreensp avatar Jun 06 '22 16:06 dgreensp

I'm seeing the same thing as @dgreensp - it appears that the imports is being passed as bundle_type because imports is being passed in the third argument in mod.ts and shows as the fourth argument in emit.generated.ts:

https://github.com/denoland/deno_emit/blob/c6f061dd8f9bb0c2f2b0709a4e9aaf657f4169b4/mod.ts#L157-L163

https://github.com/denoland/deno_emit/blob/c6f061dd8f9bb0c2f2b0709a4e9aaf657f4169b4/lib/emit.generated.js#L222-L228

I am getting this error:

error: Uncaught (in promise) Error: Unsupported bundle type "{"

I don't see any workaround - it seems that import maps don't work w/ bundle. I confirmed this isn't tested - imports is not found in https://github.com/denoland/deno_emit/blob/main/test.ts

benatkin avatar Jun 21 '22 00:06 benatkin

Yes indeed you're correct, the order of argument is broken and it isn't expected to be stringified

--- a/mod.ts
+++ b/mod.ts
@@ -157,8 +157,8 @@ export async function bundle(
   return jsBundle(
     root.toString(),
     bundleLoad,
-    JSON.stringify(imports),
-    undefined,
+    options.type,
+    imports,
     undefined,
   );
 }

It seems that some code actually handle the imports record: https://github.com/denoland/deno_emit/blob/d2b0c0ce8bd46a568278a892e339c902e493789a/wasm/src/lib.rs#L156-L166

And it is indeed passed to the bundler: https://github.com/denoland/deno_emit/blob/d2b0c0ce8bd46a568278a892e339c902e493789a/wasm/src/lib.rs#L168-L171

Which is forwarded to deno::create_graph: https://github.com/denoland/deno_emit/blob/d2b0c0ce8bd46a568278a892e339c902e493789a/rs-lib/src/lib.rs#L23-L32

But I'm pretty sure the param name is actually misleading because the following comment is displayed:

  /** An optional record of "injected" dependencies to the module graph. This
   * allows adding things like TypeScript's `"types"` values into the graph or
   * a JSX runtime. The key is the referrer specifier to use as a base when
   * resolving relative specifiers. The value is any module specifiers that are
   * being imported. */
  imports?: Record<string, string[]>;

It seems to be totally be unrelated to import map (despite the variables name being labeled as maybe_import_map in the code)

Slightly above, there's another option for deno::create_graph which is:

  /** An optional callback that allows the default resolution logic of the
   * module graph to be "overridden". This is intended to allow items like an
   * import map to be used with the module graph. The callback takes the string
   * of the module specifier from the referrer and the string URL of the
   * referrer. The callback then returns a fully qualified resolved URL string
   * specifier or an object which contains the URL string and the module kind.
   * If just the string is returned, the module kind is inferred to be ESM. */
  resolve?(specifier: string, referrer: string): string | ResolveResult;

Since it's explicitely stated "import map" in the option, I'm pretty sure it's actually this option that needs to be bound, and the imports param is useless 🤔

lowlighter avatar Sep 22 '22 23:09 lowlighter

@lowlighter

It seems to be totally be unrelated to import map (despite the variables name being labeled as maybe_import_map in the code)

This was my conclusion as well. I found that import map resolution is never done in deno_graph. It is done in the CLI. I have asked whether this logic should be moved from the CLI to deno_graph first, or reimplemented in deno_emit.

yacinehmito avatar Apr 04 '23 11:04 yacinehmito