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

Compile error when code contains export declaration-like string #3089

Closed
keroxp opened this issue Oct 8, 2019 · 26 comments
Closed

Compile error when code contains export declaration-like string #3089

keroxp opened this issue Oct 8, 2019 · 26 comments

Comments

@keroxp
Copy link
Contributor

keroxp commented Oct 8, 2019

  • 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

This comment has been minimized.

@keroxp
Copy link
Contributor Author

keroxp commented Oct 9, 2019

@nayeemrmn Oh, sorry. I edited the description.

@bartlomieju
Copy link
Member

bartlomieju commented Oct 9, 2019

@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
Copy link
Member

bartlomieju commented Oct 9, 2019

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
Copy link
Contributor

kitsonk commented Oct 9, 2019

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
Copy link
Contributor

kitsonk commented Oct 9, 2019

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
Copy link
Contributor

kitsonk commented Oct 10, 2019

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
Copy link

hsiaosiyuan0 commented Dec 7, 2019

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
Copy link
Contributor

kitsonk commented Dec 7, 2019

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
Copy link

hsiaosiyuan0 commented Dec 8, 2019

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
Copy link
Contributor

kitsonk commented Dec 8, 2019

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
Copy link
Contributor

seishun commented Feb 15, 2020

Another test case that doesn't involve string literals:

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

@kitsonk
Copy link
Contributor

kitsonk commented Feb 15, 2020

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

@seishun
Copy link
Contributor

seishun commented Feb 15, 2020

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
Copy link
Contributor

kitsonk commented Feb 16, 2020

And version of Deno?

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

@seishun
Copy link
Contributor

seishun commented Feb 16, 2020

And version of Deno?

As I said, master, which is 0.33.0.

@ry
Copy link
Member

ry commented Feb 17, 2020

fixed in #4009

@ry ry closed this as completed Feb 17, 2020
@kitsonk
Copy link
Contributor

kitsonk commented Feb 17, 2020

@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 bartlomieju reopened this Feb 17, 2020
@bartlomieju
Copy link
Member

bartlomieju commented Feb 19, 2020

@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
Copy link
Contributor

kitsonk commented Feb 19, 2020

@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
Copy link
Member

@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
Copy link
Contributor

kitsonk commented Feb 19, 2020

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
Copy link
Contributor

kitsonk commented Feb 19, 2020

(different than this issue though)

@bartlomieju
Copy link
Member

Landing #5029 might eliminate this bug altogether

@kitsonk
Copy link
Contributor

kitsonk commented May 15, 2020

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

@bartlomieju
Copy link
Member

Fixed in #5029

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

No branches or pull requests

7 participants