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

TypeScript: handle implicit cross-namespace export references #1410

Closed
osv opened this issue Jul 1, 2021 · 6 comments
Closed

TypeScript: handle implicit cross-namespace export references #1410

osv opened this issue Jul 1, 2021 · 6 comments

Comments

@osv
Copy link

osv commented Jul 1, 2021

given next source.ts:

namespace STD {
    export  function A() {
        console.log('fun A')
    }
}

namespace STD {
    export function B() {
        console.log('fun B')
        A()
    }
}

console.log(STD.B())

Compiled by ms ts tsc --outfile out-ts.js source.ts:

var STD;
(function (STD) {
    function A() {
        console.log('fun A');
    }
    STD.A = A;
})(STD || (STD = {}));
(function (STD) {
    function B() {
        console.log('fun B');
        STD.A();   // <--- Look here
    }
    STD.B = B;
})(STD || (STD = {}));
console.log(STD.B());

Compiled by esbuild esbuild source.ts --bundle:

(() => {
  // source.ts
  var STD;
  (function(STD2) {
    function A2() {
      console.log("fun A");
    }
    STD2.A = A2;
  })(STD || (STD = {}));
  (function(STD2) {
    function B() {
      console.log("fun B");
      A();   // <--- Look here, Expected: STD.A()
    }
    STD2.B = B;
  })(STD || (STD = {}));
  console.log(STD.B());
})();

Question: Is it possible to make esbuild compile like original typescript and prevent module or namespace isolation? And make possible to call function within same namespace.

@evanw
Copy link
Owner

evanw commented Jul 1, 2021

This case isn't handled correctly yet. This is just a bug/limitation of esbuild's TypeScript namespace transform. Babel's TypeScript transform doesn't handle this case either so doing what you are trying to do is already non-portable between different TypeScript implementations. These kinds of alias resolutions can get really complicated since handling this correctly in the general case involves knowing about type aliases (e.g. export type Foo = ...). Fully emulating TypeScript's type system is out of scope for esbuild due to the complexity cost, but I suspect it's still theoretically possible to handle most real-world cases with some form of limited type system special-cased to only solve for namespace aliases.

I once experimented with improving esbuild's approximation of TypeScript's namespace feature and got pretty of far but it involved a fairly large increase in complexity. I haven't landed that change yet because it was really complex, it's not finished, no one has asked for it yet, and TypeScript namespaces are somewhat of a legacy feature anyway now that ES modules exist. But I should probably try to resurrect that change and finish it at some point. Let's keep this issue open in case that ends up happening.

TL;DR: I thing it's theoretically possible to implement this in esbuild (at least in a single-file context, not across files) but it's very complex and a lot of work and isn't implemented yet. Current workarounds in the meantime are a) using fully-qualified names (e.g. just writing STD.A()) or b) just using ES modules and avoiding legacy TypeScript namespaces.

@nin-jin
Copy link

nin-jin commented Jul 1, 2021

You can simply compile to code like:

(foo.bar.A ?? foo.A ?? A)()

If it is inside namespace foo.bar.

@evanw
Copy link
Owner

evanw commented Jul 1, 2021

You can simply compile to code like:

(foo.bar.A ?? foo.A ?? A)()

If it is inside namespace foo.bar.

You are suggesting that an alias should dynamically change which export it points to at run-time depending on which exports currently have the value null or undefined. That's not how the TypeScript language works. Namespace aliases in TypeScript only ever point to a single export, and that export is statically determined at compile-time. It's valid (and common) for variables to be assigned the value null or undefined, and a transform that broke in those cases would be very incorrect.

@nin-jin
Copy link

nin-jin commented Jul 1, 2021

Do not take the direction of thought too literally.

@evanw evanw changed the title Is it possible to prevent namespace or module isolation? TypeScript: handle implicit cross-namespace export references Jul 1, 2021
@belohlavek
Copy link

belohlavek commented Jul 21, 2021

haven't landed that change yet because it was really complex, it's not finished, no one has asked for it yet

Namespaces are a neat feature for those trying to get away for the current module standard.

Would you mind expanding on the scope of the current support for namespaces on esbuild and the scope of that extra work you did?

Loved the namespace-first nature of Skew by the way! Are there any optimizations applied to namespaces in Skew that are not present in esbuild? Perhaps devirtualization/inlining optimizations?

@evanw
Copy link
Owner

evanw commented Aug 7, 2021

Would you mind expanding on the scope of the current support for namespaces on esbuild and the scope of that extra work you did?

Right now implicit references work for self and parent namespaces but not for sibling namespaces. I was experimenting with adding support for sibling namespaces within the same file but not for sibling namespaces across files, which would still not work.

Loved the namespace-first nature of Skew by the way! Are there any optimizations applied to namespaces in Skew that are not present in esbuild? Perhaps devirtualization/inlining optimizations?

Good question. I'd like to use such a feature myself if it could exist, but unfortunately JavaScript's dynamic nature makes these types of optimizations extremely hard to do correctly. For example:

namespace Foo {
  export var foo: () => void
  export var bar = 123
  foo = getFoo()
  foo()
}
console.log(Foo.bar)

In this case it's not possible to optimize foo and bar to statically-bound variables instead of dynamically-bound property accesses. It would be incorrect to compile it to this code:

// Optimizing that code to this is incorrect
var foo;
var bar = 123;
foo = getFoo();
foo();
console.log(bar);

For example, getFoo() might return function() { this.bar = null } which modifies bar via an implicit this reference. For correctness, this code must be compiled as this instead (which is what TypeScript and esbuild both do):

// That code cannot be optimized, and must generate this output
var Foo;
(function (Foo) {
  Foo.bar = 123;
  Foo.foo = getFoo();
  Foo.foo();
})(Foo || (Foo = {}));
console.log(Foo.bar);

Skew is a different language that I designed to not have this problem so that it's more optimization-friendly. In Skew there is no way to form a reference to the namespace object at run-time (it's a type, not a value) so namespaces in Skew can be a purely compile-time construct. In TypeScript namespaces unfortunately pretty much have to be a run-time construct because it's easy to form a reference to the namespace object (either via this or by storing the namespace in a variable).

There are a very limited set of circumstances where you can inline namespaces in TypeScript but the heuristics to do this correctly are very complicated and easy to accidentally disable. So I don't think something like that is worth including in esbuild itself.

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

4 participants