Disallow export-assignment modules to be imported with `import * as from` ?

[Arnavion]

While import * as foo from "foo"; compiles to var foo = require("foo"); (and equivalent for AMD) and thus works as a way to use export-assignment modules, the ES6 spec does not define this behavior. Instead the spec requires that foo be an immutable namespace object that only contains the exports of the module as properties.

Do you think it's worth disallowing the use of import * as Foo from "./foo" for export-assignment modules?

For example, if foo.d.ts is

declare module "foo" {
    function foo();
    export = foo;
}

Option 1: The compiler only allows it to be imported with import foo = require("foo"); and errors otherwise. The error message could also suggest using import foo from "foo"; with --allowSyntheticDefaultImports and a suitable loader.

Option 2: The compiler allows the ES6-style import but strips the () member, so the imported foo cannot be used as a function but other properties on it are still available. This is closer to the ES6 spec but will probably cause more user confusion. It also doesn't prevent the case where the export is of any type.

It is a breaking change, but there is precedent with the other recent breaking change to make ES6 imports immutable - that one was also based on an ES6 spec interpretation of not-necessarily-ES6 modules.

[mhegazy]

We actually did 1. initially. but the issue is most of the definitions on definitely typed are written with an export= (for reasons other than returning a class or a function for a module). and that made the ES6 module syntax unusable. so we relaxed the rule.

2 is currently in place, so your example above is actually an error today?;

 error TS2497: Module '"foo"' resolves to a non-module entity and cannot be imported using this construct.

there is a caveat though. to allow for using the destructuring import pattern on modules written with export= (again for reasons other than returning a class or a function for a module), this is allowed:

declare module "bar" {
    namespace B {
         var a;
    }
    export = B;
}
import {a} from "bar";

and as a side effect, the function call is also allowed. so:

declare module "foo" {
    function foo();
   namespace foo {} // just add a namespace
    export = foo;
}

import * as n from "foo";
n(); // OK

Ideally, we would want export= and import .. =require syntax to go away. it is not clear if and when they would though. so we are not investing in them, per se, but we can not disallow them outright as that will break a lot of code out there.

[Arnavion]

2 is currently in place, so your example above is actually an error today?

Sorry, yes you're right my example is an error without the namespace foo { } hack that you said. It's fine to be able to do import * as n from "foo"; with that hack, but n() is against ES6 spec.

Ideally, we would want export= and import .. =require syntax to go away. it is not clear if and when they would though. so we are not investing in them, per se, but we can not disallow them outright as that will break a lot of code out there.

I wasn't asking to disallow the import = require syntax, so that's fine.

[mhegazy]

with that hack, but n() is against ES6 spec.

We have talked about possibly flagging a the call expression as an error in ES6. but the feeling was it is not common, as modules move their exports to ES6, it will be harder to run into this issue.

[Arnavion]

Are you sure it's not common? A lot of existing node libraries assign functions or classes to module.exports. I'm not sure if they will all change to ES6 exports.

There is #6656 just from last week with the same issue (export-assignment of a React element). The same hack was suggested to make it work with import * as and it was tagged as a bug, instead of suggesting the correct way to use either import = require or default import + allowSyntheticDefaultImports.

Even if you don't want to make it an error, atleast don't suggest the hack instead of the correct way?

[mhegazy]

default import + allowSyntheticDefaultImports only works if your loader does the trick. e.g. SystemJS. the emitted code does not change. so i do not think it works in the #6656, unless the user is using SystemJS. which does not seem to be the case.

I think the main issue is we need to make commonJS emit/existing modules work with ES6 module syntax. i think things will become clearer when node have an ES6 module implementation along with some back-compat story.

A true complaint ES6 module implementation would not have this issue anyways. so again it is only the interop issue, which is not clear yet, as there is no ES6 implementation yet.

[Arnavion]

@mhegazy Why did you close this?

[RyanCavanaugh]

@Arnavion seems like you had a question that was answered. I think the best path forward would be a separate issue if you think there's something concrete that should happen.

[Arnavion]

@RyanCavanaugh It was not a question. I asked for a change in the OP.

[Arnavion]

To clarify, @mhegazy said that it's pending a real-life implementation of ES6 + ES5 modules interop to decide whether import * as foo from "foo" is legal where foo is an ES5 module. I can understand that, so it's okay that foo.bar() is legal.

However it's still certain that foo() is an error, since the ES6 spec is unambiguous on the fact that foo is a ModuleNamespaceObject. So that should be disallowed to prevent users from writing ES6 code that's invalid ES6.

[RyanCavanaugh]

I think Mohamed addressed why we didn't do that

[Arnavion]

So just to confirm, you're saying that it's okay that TS allows users to write ES6 code that's invalid ES6 ?

[mhegazy]

We should file an issue to issue an error for calling a namespace import.