Compiler allows for using a value before it is imported

[lambda-fairy]

TypeScript Version: 2.3.4

Code

// foo.ts
console.log(BAR);
import {BAR} from './bar';

// bar.ts
export const BAR = 'bar';

// tsconfig.json
{
  "compilerOptions": {
    "target": "es5",
    "module": "commonjs",
    "strict": true
  }
}

Expected behavior:

TypeScript should reject foo.ts, because it is referring to BAR before it is initialized.

Actual behavior:

tsc raises no errors, and generates the following foo.js file:

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
console.log(bar_1.BAR);
var bar_1 = require("./bar");

This fails at runtime with a TypeError.

[lambda-fairy]

Note that when I first encountered this issue, it was in code like this:

// ...
import BAR = innerNamespace.BAR;
// ...
import {innerNamespace} from './bar';
// ...

So it wasn't as obvious what was wrong.

[yortus]

ES6 imports are hoisted, so the code in foo.ts is valid and should run without errors. The problem here seems to be with the emitted code, which does not hoist the import. Not sure if changing the emit order would be problematic when emitting to other module systems like CommonJS.

[yortus]

FWIW I was curious what babel emits for foo.ts, and it correctly hoists the import, emitting the following JS code (see repl here):

'use strict';

var _bar = require('./bar');

console.log(_bar.BAR);

[lambda-fairy]

That's interesting, thanks for the info!

I guess changing the emit order would technically be a breaking change, since importing a module can have side effects. But I'm not sure how common this is in practice.

[rbuckton]

Ideally we would hoist the imports, but not change the emit order. This is what already happens for AMD and SystemJS.

[appsforartists]

I just found a similar surprise, and a place where TS should throw and doesn't:

function useA() {
  // using `a` before it is declared surprisingly doesn't error in either TS or the 
  // DevTools console
  console.log(a);
}

useA(); // This does error at run time and should error at compile time

const a = 1;
useA();

[scottmas]

It looks like this issue is scheduled to be closed soon with #39764 which tweaks import hoisting, but this fix will NOT fix the issue described by @appsforartists, which is the same bug I've experienced in my code.

Are there plans to fix this error?

[uhyo]

I proposed a fix two years ago, but it got closed today. It never received any piece of reviews.

I must say that apparently TypeScript Team is not interested in fixing this issue.

Please do not expect this issue to be fixed. 🥲

[RyanCavanaugh]

Unfortunately, surveying the landscape, there are definitely projects that have taken a hard dependency on the current behavior and would by broken by any fix to hoist the imports, e.g. https://github.com/microsoft/tsdoc/blob/b7bddc1ceb6b6759857c66ef3af6e5e95655fb8c/tsdoc/src/nodes/DocDeclarationReference.ts#L165 where the lines are reordered to prevent a circular dependency problem.

Since writing imports below the top of the file is extremely rare (seems like only book and transpiler authors do this 😅) we don't think this is worth potentially breaking people for the sake of a spec compliance that no one encounters in practice.

[appsforartists]

@RyanCavanaugh
Is this worth elevating to its own issue?

[RyanCavanaugh]

@appsforartists that behavior is intentional. Many programs "look" like they will TDZ under static analysis, but don't at runtime, so it's not practical to flag those as errors

[magic-akari]

@RyanCavanaugh

It's interesting to see this example.
Babel is correct, we should mimic esm behavior and hoist import statements since it's valid esm specific syntax.

For the case of tsdoc, commonjs specific syntax should be used.

something like

// Circular reference
const { TSDocEmitter } = require('../emitters/TSDocEmitter');

or

// Circular reference
import _TSDocEmitter  = require('../emitters/TSDocEmitter');
const { TSDocEmitter } = _TSDocEmitter;