Import statements are hoisted when target is CommonJs

[petkaantonov]

Describe the bug

Import statements are hoisted when target is CommonJS:

import A from "a;
console.log(1);
import B from "a";

Becomes

require(...);
require(...);
console.log(1);

This is incorrect, require calls are normal functions and should have preserved order. Imports should only be hoisted when they are actual es6 imports. This is incompatible with tsc and probably others, tsc keeps the order as expected when output module is commonjs.

Additional context

It was fixed but then reverted in #1457

[mischnic]

With native esm (in Node or in browsers), the following prints "a, b, index"

// index.js
import "./a.js";
console.log("index");
import "./b.js";

// a.js
console.log("a");

// b.js
console.log("b");

And I'd argue that transpiling the code to CJS shouldn't change the behaviour of the code

[petkaantonov]

• It is incompatible with tsc
• It sucks for commonjs users, remember we are forced to use import not because it is better syntax but because typescript types dont work with require
• Most code was written with require in mind, so require order matters
• If you are able to use esm, why compile code to cjs? and why expect the behavior be the same? They are incompatible module systems.

[mischnic]

From my perspective, the real problem is that tsc and Babel/the spec don't agree

I guess swc has to choose a side here.

[kdy1]

This feature is useful for using some of node.js modules, which expect execution order of common js modules.
So I think this behavior should be allowed, but only with a bad-named flag, just like dangerouslySetInnerHTML.
I'll think more about the name of the flag.

[petkaantonov]

Some suggestions: disableImportHoisting, noImportHoist

[devongovett]

Isn't this exactly what the jest mock transform allows already? Do you have a different use case for this? IMO deviating from the spec behavior is a bad idea. TSC is simply wrong.

If you want to imports to behave like require, why not just write require?

[kdy1]

I missed that users can use require.
Thanks, @devongovett !

I think users should use require if the execution order of require is required.

[devongovett]

Also fwiw TSC is planning on changing this. microsoft/TypeScript#39764

[petkaantonov]

Isn't this exactly what the jest mock transform allows already? Do you have a different use case for this? IMO deviating from the spec behavior is a bad idea. TSC is simply wrong.

If you want to imports to behave like require, why not just write require?

TypeScript types don't work with require. When I am compiling TypeScript to CommonJS, the ESM spec has nothing to do with it and its spec should be irrelevant and the commonjs spec should be relevant - which specifies no hoisting. At least a flag should be introduced so that commonjs spec is followed when compiling for commonjs.

[lagnat]

Also fwiw TSC is planning on changing this. microsoft/TypeScript#39764

Apparently not. The PR was closed and the bug is marked won't-fix.

[kdy1]

@lagnat Please see #5205

[lagnat]

We could argue if it's a tsc bug or not but we won't convince each other. What is more at issue is that some projects that use tsc cannot safely switch to swc without re-writes. You could add a compatibility flag to get more projects using swc but it seems that the swc teams is not interested in that and I'm here to voice my opinion that I find it unfortunate.

[magic-akari]

@lagnat evanw's explanation is the best answer.

The conversion from ESM to CommonJS done by esbuild attempts to preserve the behavior of the original code as much as possible. What you are observing happens because in ESM, all imports are essentially "hoisted" to the top of the file since all dependencies of a given file must be evaluated before any code in that file is evaluated. This is just how the JavaScript language works. More information: evanw/esbuild#1395 (comment).

The code you wrote happens to work due to a defect in the TypeScript compiler: microsoft/TypeScript#16166 (comment). Writing such code is risky because your code will break as soon as it's run in a real ESM environment (such as esbuild, or node's ESM mode, or in a browser).

Originally posted by @evanw in evanw/esbuild#1444 (comment)

[lagnat]

It's not a defect because Microsoft closed the bug report as won't-fix and didn't take the PR. Microsoft gets to decide how tsc works and this is what they've decided. I get that you don't agree with the decision and I understand why.. in fact I totally agree with you, but tsc works the way it works and I don't see it changing. I fully acknowledge that the authors of swc can do whatever they want too but it's a strange hill to die on because it hinders a wider adoption of swc. Adding a config option such as emulateBuggyTscAndDontHoist makes your position clear and gets more code on swc. I don't use esm at the moment and I acknowledge that I'll be forced to deal with this issue when that time comes.

[kdy1]

Please use discussion thread instead