Legacy importing type from namespace is not supported

[JounQin]

import { Rule, Scope } from 'eslint'; // `Rule` and `Scope` are both ts namespace
import Variable = Scope.Variable;
import CodePathSegment = Rule.CodePathSegment;
import Variable = Scope.Variable;
                        ^

TypeError: Cannot read properties of undefined (reading 'Variable')

related privatenumber/tsx#83

[hyrious]

I'm not sure whether esbuild should treat both Variable and Scope as just type, because import A = B.C can actually be compiled to const A = B.C, which may invoke a get C() {} and has side effects.

Here's the babel behavior:

// input
import { Scope } from 'eslint'
import Variable = Scope.Variable
let a: Variable

// output
var Variable = Scope.Variable;
let a;
export {};

esbuild:

// input
import { Scope } from 'eslint'
import Variable = Scope.Variable
let a: Variable

// output
import { Scope } from "eslint";
const Variable = Scope.Variable;
let a;

esbuild, but annotate Variable as a type, but note that this is actually a type error in typescript:

// input
import { Scope } from 'eslint'
import type Variable = Scope.Variable
let a: Variable

// output
let a;

Interesting, sucrase seems to have done what you want:

// input
import { Scope } from 'eslint'
import Variable = Scope.Variable
let a: Variable

// output
let a;

[JounQin]

It's about compatibility with TypeScript itself

import type Variable = Scope.Variable

it's invalid TypeScript syntax AFAIK.

[hyrious]

it's invalid TypeScript syntax AFAIK.

Yep, I've just updated comments above.

So It's hard to say which is right here, maybe babel is wrong too.

[JounQin]

So It's hard to say which is right here, maybe babel is wrong too.

Yep, I'm going raise an issue on Babel also.

[evanw]

One unusual thing about esbuild is that by default, the transform API does not assume that the provided input is a complete module. This allows for you to either run the output in the global scope or to add additional code after the output to form a module. For example, this can be used to implement TypeScript support for Svelte: #604. So this import could hypothetically be of a variable, not a type, in which case it would need to be kept in this scenario.

However, I should be able to make this work if you enable tree shaking, either explicitly with treeShaking: true or implicitly with bundle: true. The idea would be to mark the property accesses in an import assignment expression as side-effect free, which would allow esbuild to remove them. Currently esbuild treats them as potentially having side effects because they are potentially values, not types, in which case the property access expression evaluates code. I think it should be ok to consider these to be side-effect free because these are a TypeScript construct that's supposed to be used with namespaces, and namespaces don't have any syntax for exporting getters that might have side effects.

[evanw]

It's about compatibility with TypeScript itself

import type Variable = Scope.Variable

it's invalid TypeScript syntax AFAIK.

If you only want a type, you can just use type Variable = Scope.Variable right? And you can use const Variable = Scope.Variable if you only want a value. I thought the only reason you need import Variable = Scope.Variable is if Variable is both a type and a value.

[JounQin]

@evanw The problem is the source codes are out controlled from us. 😅I'm using a git submodule.

[privatenumber]

I thought the only reason you need import Variable = Scope.Variable is if Variable is both a type and a value.

Doesn't this mean that import x = y is not exactly side-effect free?

This behavior is a little strange to me because in the following code snippet, if someNamespace is a type, it will resolve to being undefined at runtime, and const bar = someNamespace.foo will throw a Type Error about reading a property on undefined. Which I consider a side-effect.

However, when tree-shaking enabled, the erroneous code is removed and the module is "fixed".

import { someNamespace } from './some-file'
import bar = someNamespace.foo;

I should note this is only a problem when transforming to CJS. In ESM, the import statement will assert that someNamespace is exported, and when it's not, it can be fixed via type import. Perhaps the problem here is that the CJS transformation doesn't assert someNamespace to be exported.

[evanw]

When in one-file-at-a-time mode, the official TypeScript compiler always removes bar completely if no code in the file uses it: (TypeScript playground link). So in that sense it's side-effect free.

Technically I suppose I should also remove the import statement itself if all import assignments are removed, so I guess I could reopen this issue for that purpose. Removing an import is different than tree shaking an unused variable though. So this would mean changing esbuild to just always do the removal independent of whether tree shaking is enabled or not.