Deno: Compile error when code contains export declaration-like string

[keroxp]

• v0.20.0
• Parser doesn't find declarations correctly?

[EDITED]

Previous example has unrelated mistake but I don' know which code can reproduce error I met.

I met this error when I ran -> https://raw.githubusercontent.com/keroxp/dink/v0.5.1/main.ts
That can be compiled with [email protected]. But got error below with v0.20.0:

error: Uncaught ImportPrefixMissing: relative import path "${resp.url}" not prefixed with / or ./ or ../
► $deno$/dispatch_json.ts:40:11
    at DenoError ($deno$/errors.ts:20:5)
    at unwrapResponse ($deno$/dispatch_json.ts:40:11)
    at sendAsync ($deno$/dispatch_json.ts:85:10)

[nayeemrmn]

Hmm, there doesn't seem to be a problem? ${deno} in a template literal gets substituted with the value assigned to variable deno, which you haven't declared.

[keroxp]

@nayeemrmn Oh, sorry. I edited the description.

[bartlomieju]

@keroxp I can take a look later, can you provide repro commands?

EDIT: Nvm, running deno https://raw.githubusercontent.com/keroxp/dink/v0.5.1/main.ts gives this error. I'll investigate it :)

[bartlomieju]

Interesting bug @keroxp!

I looks like output from ts.preProcessFile() in cli/js/compiler.ts return this template string as an actual import.

// https://raw.githubusercontent.com/keroxp/dink/v0.5.1/main.ts
...
   let link = `export * from "${resp.url}";\n`;
    if (hasDefaultExport) {
      link += `import {default as dew} from "${resp.url}";\n`;
      link += `export default dew;\n`;
    }
    await Deno.mkdir(modDir, true);
...

$ deno_dev https://raw.githubusercontent.com/keroxp/dink/v0.5.1/main.ts
Compile file:///Users/biwanczuk/dev/dink/main.ts
preprocessed file imports [ 
{ fileName: "./vendor/https/deno.land/std/fs/path.ts", pos: 23, end: 62 }, 
{ fileName: "./vendor/https/deno.land/std/fs/mod.ts", pos: 86, end: 124 }, 
{ fileName: "./vendor/https/deno.land/std/flags/mod.ts", pos: 151, end: 192 }, 
{ fileName: "./vendor/https/deno.land/std/fmt/colors.ts", pos: 229, end: 271 }, 
{ fileName: "${resp.url}", pos: 3914, end: 3925 }, 
{ fileName: "${resp.url}", pos: 4005, end: 4016 } 
]
error: Uncaught ImportPrefixMissing: relative import path "${resp.url}" not prefixed with / or ./ or ../
► $deno$/dispatch_json.ts:40:11
    at DenoError ($deno$/errors.ts:20:5)
    at unwrapResponse ($deno$/dispatch_json.ts:40:11)
    at sendAsync ($deno$/dispatch_json.ts:85:10)

Calling @kitsonk for help

EDIT: IMHO this is a TypeScript bug - it shouldn't treat imports inside template string as an actual import in the file.

@keroxp a quick workaround for now would be to replace template strings and just add strings old-school (str1 + str2).

[kitsonk]

I suspect it is something with preprocess, and preprocess potentially related to how we analyse the @deno-types which could accidentally detect import/exports that way. If it isn't our regex that detects those, then it will be an error with the preprocess and the TypeScript tokenizer (which I doubt, but who knows). I should get time today though.

[kitsonk]

Actually, thinking through it, it has to be an issue with preprocess, or with how we are using preprocess. It can't be the type directives.

[kitsonk]

Yeah, it is a TypeScript issue with preProcessFile. Funnily, it was already open and tagged as a bug, but the reverse. It seems string literals with a variable interpolation in them cause the parser to flip modes, not detecting import/export statements outside of string literals, and detecting them within string literals. I can't see any easy way to fix it right now. The workaround might be the best thing until the bug is fixed.

Refs: microsoft/TypeScript#30878

[hsiaosiyuan0]

I glanced the implementation of ts.preProcessFile it just do something like a regular expression does, so it's quick but missing some correctness because the syntax rules is a recursive structure and not very easy being processed via a linear process.

I don't know why preProcessFile is designed like that maybe performance is its first target? The frequency of this bug is relatively lower but the error level is high, so I think it would be better if we could provide a fixture if the upstream's response is slow.

[kitsonk]

I don't know why preProcessFile is designed like that maybe performance is its first target?

Yes, correct.

As far as fixing the bug, I am sure the TypeScript team would welcome the contribution.

[hsiaosiyuan0]

The first target in our case is the correctness rather than performance in their situation. Although the preProcessFile does some wrong but it does not mess the main process of typescript, introduce this api in Deno cause a normal program cannot bootstrap.

As far as fixing the bug, I am sure the TypeScript team would welcome the contribution.

Good to know that, thank you

[kitsonk]

Actually, we introduced it to improve our performance and allow the whole of the TypeScript compiler to operate in an asynchronous fashion. There is a work around, to not use string literals that contain something that looks like an import statement.

[seishun]

Another test case that doesn't involve string literals:

function define(foo) {}
define(["long"]);

[kitsonk]

@seishun please provide more information, like that actual error output and the version of Deno you are using.

[seishun]

master, same error output:

Compile file:///C:/Users/Nikolai/deno/test.ts
error: Uncaught ImportPrefixMissing: relative import path "long" not prefixed with / or ./ or ../ Imported from "file:///C:/Users/Nikolai/deno/test.ts"

[kitsonk]

And version of Deno?

Note to self, check if preprocess analyses AMD type of structures and if there is anyway to modify that.

[seishun]

And version of Deno?

As I said, master, which is 0.33.0.

[ry]

fixed in https://github.com/denoland/deno/pull/4009

[kitsonk]

@ry no this only fixed part of the issue. String literal with export like statements are still awaiting a fix in TypeScript. Please re-open.

[bartlomieju]

@kitsonk, not sure if that's related, but after #4030 I tried to run jsdom and got:

// foo.ts
window["global"] = window;
import "https://dev.jspm.io/jsdom";


$ ./target/debug/deno -r foo.ts
...
error: Uncaught ImportPrefixMissing: relative import path "exports" not prefixed with / or ./ or ../ Imported from "https://dev.jspm.io/npm:[email protected]/dist/acorn.dew.js"
► $deno$/dispatch_json.ts:40:11
    at DenoError ($deno$/errors.ts:20:5)
    at unwrapResponse ($deno$/dispatch_json.ts:40:11)
    at sendSync ($deno$/dispatch_json.ts:67:10)
    at resolveModules ($deno$/compiler_imports.ts:71:10)
    at processImports ($deno$/compiler_imports.ts:166:27)
    at processImports ($deno$/compiler_imports.ts:175:13)

Can't really pin-point what's the problem here, but it looks a lot like something's treated as import statement while it shouldn't.

[kitsonk]

@bartlomieju what does -L debug provide? But why is the compiler involved in that at all? It shouldn't be sending JavaScript to the compiler unless you have checkJs enabled, which you don't. Seems like somehow there was a regression there.

[bartlomieju]

@kitsonk here's the full output: https://gist.github.com/bartlomieju/10fdab8ee91665b3f802f4b5711dc5b8
It's run through TS compiler cause I created a file called foo.ts; here's full contents:

window["global"] = window
import "https://dev.jspm.io/jsdom";

The first line is added because jsdom complains that global is not defined.

If I change the file to foo.js then I immediately get error about missing global:

error: Uncaught ReferenceError: global is not defined
► https://dev.jspm.io/npm:@jspm/[email protected]/nodelibs/process.js:4:52

4 var _global = typeof self !== "undefined" ? self : global;
                                                     ^

    at https://dev.jspm.io/npm:@jspm/[email protected]/nodelibs/process.js:4:52

[kitsonk]

Ok, so what is triggering it is the preprocess file, and imports. Basically we shouldn't analyse dependencies on JS modules if checkJS isn't turned on, but we always do. I am working on a patch.

[kitsonk]

(different than this issue though)

[bartlomieju]

Landing #5029 might eliminate this bug altogether

[kitsonk]

_should_ eliminate, I mean it would be a full AST parse so it is unlikely to persist.

[bartlomieju]

Fixed in #5029