Deno.emit should correct local import URLs

[KSXGitHub]

Problem

Deno.compile() does not convert local import URLs from .ts extension to .js extension, making it impossible to use inside browser and Node.js.

Steps to reproduce

Run this file (deno run https://gist.githubusercontent.com/KSXGitHub/c94a9d7eb8e976f1126b4b9dfeba0497/raw/004a9ddb6872b9a2f05506f13f3efdfd6edd6d69/tmp.ts).

Expected behavior

1. All local import URLs in output JS files should have .js extension.

// foo.js should look like this
export * from "./bar.js";

2. All local import URLs in output TypeScript definition files (.d.ts) should have no extension.

// foo.d.ts should look like this
export * from "./bar";

Actual behavior

All local import URLs are preserved, which is incorrect.

[kitsonk]

Hmmm... guessing at this could be problematic. I think we are going to have to introduce some options to control this, versus having a distinct opinion on it.

[KSXGitHub]

@kitsonk I think that option should be a function in JavaScript API, for example:

Deno.compile(root, sources, {
  transformImportURL: info => info.outputFile.endsWith('.js')
    ? info.url.replace(/\.ts$/, '.js')
    : info.url.replace(/\.ts$/, '')
})

[kitsonk]

I thought about that initially, but the problem is that the compilation actually runs in another isolate. We could do that processing back in the runtime isolate, but that would add specific logic that would be hard to follow.

[bartlomieju]

@kitsonk I think we've discussed this issue recently and came to a conclusion that it's a "won't fix"?

[kitsonk]

Well, let's keep it open until after we address denoland/deno#4752. I can see how it can be a problem/challenge. Obviously the best thing to do at the moment is Deno.bundle() instead of Deno.compile().

[Eyal-Shalev]

@kitsonk Could you update the issue title and description to represent the change from Deno.compile to Deno.emit. I opened denoland/deno#10272 because I couldn't find this issue.

[Eyal-Shalev]

@kitsonk Also, is there an issue (or discussion page) where the source of this problem is explained?

[kitsonk]

@Eyal-Shalev neither tsc nor swc modify module specifiers. Actually I think we might be able to get swc to do it, but it also means guessing at what the right answer is for the target.

[Eyal-Shalev]

@kitsonk what if sources contained the desired module name in addition to the source file?

type Source = string | { name: string, data: string }
interface EmitOptions {
  sources: Record<string, Source>
}

[kitsonk]

It isn't that straight forward... tsc refuses to touch the module specifier at all (there are a few issues where they make that clear) which then leaves it to swc. It isn't just a "find and replace", because module specifiers can be relative, so it has to be parsed to a symbol, the symbols resolved and then during the emit process the new "target" specifier replace it. To do it properly and fool proof for all situations is somewhat complex. Someone doing a post processing step themselves, is actually far easier, because they can make sure it fits their specific use case and context.

[Ciantic]

I'm doing an ES module library which should be usable from browser as well as from deno, e.g.:

math.ts -> math.js thing.ts -> thing.js

So browsers can import from js files and deno from ts files. With TSC this is somewhat straightforward but not so much with deno.

Turns out that is not possible with deno because of this. It leaves the import statements like this after emit:

import { v3, vDot } from "./math.ts

Thus they don't work with browsers. I guess I have to regexp all .ts" with .js" and hope for the best.

[ebebbington]

@Ciantic (and anyone else really), you can add a temp fix:

private static correctExtensionsOnImportsInFile(fileContent: string): string {
    // Because imports inside the files are still using a .ts extension, we're going to make them use .js:
    fileContent = fileContent.replace(
      /import { ([a-zA-Z].*) } from "(.*)";/gm,
      (_str, importValues, fileImportedFrom) => {
        const jsImport = fileImportedFrom.replace(".ts", ".js");
        return `import { ${importValues} } from \"${jsImport}\";`;
      },
    );
    fileContent = fileContent.replace(
      /export { ([a-zA-Z].*) } from "(.*)";/gm,
      (_str, importValues, fileImportedFrom) => {
        const jsImport = fileImportedFrom.replace(".ts", ".js");
        return `export { ${importValues} } from \"${jsImport}\";`;
      },
    );
    fileContent = fileContent.replace(
      /import \"(.*)\";/gm,
      (_str, importValue) => {
        const jsRef = importValue.replace(".ts", ".js");
        return `import \"${jsRef}\";`;
      },
    );
    fileContent = fileContent.replace(
      /import\(\"(.*)\"\)/gm,
      (_str, importValue) => {
        const jsRef = importValue.replace(".ts", ".js");
        return `import(\"${jsRef}\")`;
      },
    );
    fileContent = fileContent.replace(
      /import (.*) from \"(.*)\"/gm,
      (_str, importValue, url) => {
        const jsRef = url.replace(".ts", ".js");
        return `import ${importValue} from \"${jsRef}\"`;
      },
    );
    return fileContent;
  }

Though that could probably be made simpler