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

import * as alias syntax doesn't work with export = function unless merged with namespace #5073

Closed
weswigham opened this Issue Oct 2, 2015 · 17 comments

Comments

Projects
None yet
8 participants
@weswigham
Member

weswigham commented Oct 2, 2015

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

Can't be included via import * as f from "foo";.
(test.ts(1,20): error TS2497: Module '"foo"' resolves to a non-module entity and cannot be imported using this construct.)
Whereas

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

Can be. The distinction seems artificial.

@mhegazy

This comment has been minimized.

Show comment
Hide comment
@mhegazy

mhegazy Oct 2, 2015

Contributor

there is no way in an ES6 module to achieve this. the parallel in an ES6 module world is to default export. import * will import the namespace component of the export, if it exists, if it does not, it is an error. That you can call the alias as a function in the second case, is the bug i would say.

Contributor

mhegazy commented Oct 2, 2015

there is no way in an ES6 module to achieve this. the parallel in an ES6 module world is to default export. import * will import the namespace component of the export, if it exists, if it does not, it is an error. That you can call the alias as a function in the second case, is the bug i would say.

@mhegazy mhegazy added the By Design label Oct 2, 2015

@mhegazy mhegazy closed this Oct 2, 2015

@horiuchi horiuchi referenced this issue Jan 4, 2016

Merged

fix import #7417

sventschui added a commit to sventschui/DefinitelyTyped that referenced this issue Jan 31, 2016

borislavjivkov added a commit to borislavjivkov/DefinitelyTyped that referenced this issue Feb 3, 2016

liushigit added a commit to liushigit/DefinitelyTyped that referenced this issue Apr 14, 2016

added an empty namespace in koa-bodyparser.d.ts
Without this, there will be `Module '"koa-bodyparser"' resolves to a non-module entity and cannot be imported using this construct. (2497)` error
This solution is found (here)[Microsoft/TypeScript#5073].
koa-json's `.d.ts` file has such namespace.

horiuchi added a commit to DefinitelyTyped/DefinitelyTyped that referenced this issue Apr 14, 2016

added an empty namespace in koa-bodyparser.d.ts (#8979)
Without this, there will be `Module '"koa-bodyparser"' resolves to a non-module entity and cannot be imported using this construct. (2497)` error
This solution is found (here)[Microsoft/TypeScript#5073].
koa-json's `.d.ts` file has such namespace.

vvakame added a commit to DefinitelyTyped/DefinitelyTyped that referenced this issue Apr 14, 2016

added an empty namespace in koa-bodyparser.d.ts (#8979)
Without this, there will be `Module '"koa-bodyparser"' resolves to a non-module entity and cannot be imported using this construct. (2497)` error
This solution is found (here)[Microsoft/TypeScript#5073].
koa-json's `.d.ts` file has such namespace.

vvakame added a commit to DefinitelyTyped/DefinitelyTyped that referenced this issue Apr 17, 2016

Missing locale functions and types (#8981)
* added svgRendering property to Html2CanvasOptions interface

* added an empty namespace in koa-bodyparser.d.ts (#8979)

Without this, there will be `Module '"koa-bodyparser"' resolves to a non-module entity and cannot be imported using this construct. (2497)` error
This solution is found (here)[Microsoft/TypeScript#5073].
koa-json's `.d.ts` file has such namespace.

* Missing locale functions and types

tkrotoff added a commit to tkrotoff/DefinitelyTyped that referenced this issue May 21, 2016

vvakame added a commit to DefinitelyTyped/DefinitelyTyped that referenced this issue May 24, 2016

Allow ES6 import syntax for wolfy87-eventemitter.d.ts (#9415)
* Boostrap -> Bootstrap

* Allow ES6 import syntax for wolfy87-eventemitter.d.ts

See Microsoft/TypeScript#5073

garthk added a commit to garthk/DefinitelyTyped that referenced this issue Jul 26, 2016

Fix error TS2497 on `import * as X from 'statsd-client'`:
Per Microsoft/TypeScript#5073, closed as `By Design` by @mhegazy,
we need to merge a namespace with the export when using `export =`,
otherwise `import *` must fail.

garthk added a commit to garthk/DefinitelyTyped that referenced this issue Jul 26, 2016

Fix error TS2497 on import * as X from 'statsd-client':
Per Microsoft/TypeScript#5073, closed as `By Design` by @mhegazy, we need to
export a namespace for `import *` to work, else `TS2497`. That clashes with
the `export = ClassName` pattern unless you also merge in a namespace, e.g.
with `namespace ClassName {}`.

garthk added a commit to garthk/DefinitelyTyped that referenced this issue Aug 9, 2016

Fix error TS2497 on import * as X from 'statsd-client':
Per Microsoft/TypeScript#5073, closed as `By Design` by @mhegazy, we need to
export a namespace for `import *` to work, else `TS2497`. That clashes with
the `export = ClassName` pattern unless you also merge in a namespace, e.g.
with `namespace ClassName {}`.

garthk added a commit to garthk/DefinitelyTyped that referenced this issue Aug 9, 2016

Fix error TS2497 on import * as X from 'statsd-client':
Per Microsoft/TypeScript#5073, closed as `By Design` by @mhegazy, we need to
export a namespace for `import *` to work, else `TS2497`. That clashes with
the `export = ClassName` pattern unless you also merge in a namespace, e.g.
with `namespace ClassName {}`.

ramonornela added a commit to ramonornela/DefinitelyTyped that referenced this issue Aug 14, 2016

@blakeembrey

This comment has been minimized.

Show comment
Hide comment
@blakeembrey

blakeembrey Aug 26, 2016

Contributor

@mhegazy Have you considered enabling a flag for CommonJS interop in ES6 modules? It's inevitable that people will want to use CommonJS requires and have ES6 exports for a period of time.

Edit: To clarify, in case that didn't make sense, a flag that allows import x = require('x') syntax when using ES6 exports. Having every node module update now, or into the future, seems pretty unfeasible so even people that will adopt ES6 (now with their bundler or in the future with native support) will require to continue using import x = require('x') for "legacy" external modules.

Contributor

blakeembrey commented Aug 26, 2016

@mhegazy Have you considered enabling a flag for CommonJS interop in ES6 modules? It's inevitable that people will want to use CommonJS requires and have ES6 exports for a period of time.

Edit: To clarify, in case that didn't make sense, a flag that allows import x = require('x') syntax when using ES6 exports. Having every node module update now, or into the future, seems pretty unfeasible so even people that will adopt ES6 (now with their bundler or in the future with native support) will require to continue using import x = require('x') for "legacy" external modules.

@mhegazy

This comment has been minimized.

Show comment
Hide comment
@mhegazy

mhegazy Aug 27, 2016

Contributor

what is the expected emit for import x = require("x") in ES6?

Contributor

mhegazy commented Aug 27, 2016

what is the expected emit for import x = require("x") in ES6?

@blakeembrey

This comment has been minimized.

Show comment
Hide comment
@blakeembrey

blakeembrey Aug 27, 2016

Contributor

I would say var x = require('x'), as you assume people using this would have a CommonJS-capable bundler and are already doing this with say, Babel. That said, in the future it would be possible to align this with the node-proposal as I imagine they'll have interop as well - it's unlikely node will drop an entire ecosystem of hundreds of thousands of modules when adding support for ES6 modules.

Contributor

blakeembrey commented Aug 27, 2016

I would say var x = require('x'), as you assume people using this would have a CommonJS-capable bundler and are already doing this with say, Babel. That said, in the future it would be possible to align this with the node-proposal as I imagine they'll have interop as well - it's unlikely node will drop an entire ecosystem of hundreds of thousands of modules when adding support for ES6 modules.

@unional

This comment has been minimized.

Show comment
Hide comment
@unional

unional Apr 4, 2017

Contributor

Without the hack, TS code cannot target es6 if it consumes cjs module.

With Babel, JS user is doing import x from 'x' and it is working fine for them.
For TS, right now we do not have any working solution except the hack.

Contributor

unional commented Apr 4, 2017

Without the hack, TS code cannot target es6 if it consumes cjs module.

With Babel, JS user is doing import x from 'x' and it is working fine for them.
For TS, right now we do not have any working solution except the hack.

@byrgvt

This comment has been minimized.

Show comment
Hide comment
@byrgvt

byrgvt May 3, 2017

I still can't tell if this was ever intended behavior or not (that using the namespace allows this to pass typecheck), or just a happy accident that now too many people rely on to change.n

It seems an empty namespace gets SymbolType NamespaceModule (1024) and the checker looks for the merged symbol. So basically

function C {} // symbol type 16
namespace C {} // symbol type 1024
(1024 | 16) & SymbolFlags.Module == true (from resolveESModuleSymbol in compiler/checker.ts)

is what I think is happening?

The PR that added module support #2242 explicitly mentions:

A module that uses export = to export a non-module entity in place of the module itself must be imported using the existing import x = require("foo") syntax as is the case today.

But I don't understand what typescript considers a non-module entity. The actual line in checker that flags these errors is:
(!dontResolveAlias && symbol && !(symbol.flags & (SymbolFlags.Module | SymbolFlags.Variable)))

So I can get away with pretty confusing stuff like:

a.ts
const x = () => 1;
export = x;
b.ts
import * as x from './a';

x()

Which passes compilation fine as with ./node_modules/.bin/tsc b.ts -t "es6" -m "amd".

Is a module entity related to https://www.ecma-international.org/ecma-262/6.0/#sec-module-namespace-exotic-objects, which is what the es6 grammar specifies is the type of import * as foo?

byrgvt commented May 3, 2017

I still can't tell if this was ever intended behavior or not (that using the namespace allows this to pass typecheck), or just a happy accident that now too many people rely on to change.n

It seems an empty namespace gets SymbolType NamespaceModule (1024) and the checker looks for the merged symbol. So basically

function C {} // symbol type 16
namespace C {} // symbol type 1024
(1024 | 16) & SymbolFlags.Module == true (from resolveESModuleSymbol in compiler/checker.ts)

is what I think is happening?

The PR that added module support #2242 explicitly mentions:

A module that uses export = to export a non-module entity in place of the module itself must be imported using the existing import x = require("foo") syntax as is the case today.

But I don't understand what typescript considers a non-module entity. The actual line in checker that flags these errors is:
(!dontResolveAlias && symbol && !(symbol.flags & (SymbolFlags.Module | SymbolFlags.Variable)))

So I can get away with pretty confusing stuff like:

a.ts
const x = () => 1;
export = x;
b.ts
import * as x from './a';

x()

Which passes compilation fine as with ./node_modules/.bin/tsc b.ts -t "es6" -m "amd".

Is a module entity related to https://www.ecma-international.org/ecma-262/6.0/#sec-module-namespace-exotic-objects, which is what the es6 grammar specifies is the type of import * as foo?

@qm3ster

This comment has been minimized.

Show comment
Hide comment
@qm3ster

qm3ster Dec 5, 2017

So, is this the hack?
And is it still the best way?

declare module 'nedb-core' {
	class Datastore {
		constructor(options?: {})
	}
	namespace Datastore {

	}
	export = Datastore
}

qm3ster commented Dec 5, 2017

So, is this the hack?
And is it still the best way?

declare module 'nedb-core' {
	class Datastore {
		constructor(options?: {})
	}
	namespace Datastore {

	}
	export = Datastore
}
@LouisWayne

This comment has been minimized.

Show comment
Hide comment
@LouisWayne

LouisWayne Dec 25, 2017

@qm3ster I think so.. I don't like the consistency here..have you found any solution for this???

@qm3ster I think so.. I don't like the consistency here..have you found any solution for this???

@qm3ster

This comment has been minimized.

Show comment
Hide comment
@qm3ster

qm3ster Jan 3, 2018

@LouisWayne well, the reason a hack might be the best we can do is because we are trying to ES6-import something that could never have been ES6-exported.
I am currently doing what I wrote above and not having any problems.

qm3ster commented Jan 3, 2018

@LouisWayne well, the reason a hack might be the best we can do is because we are trying to ES6-import something that could never have been ES6-exported.
I am currently doing what I wrote above and not having any problems.

claytonsilva added a commit to claytonsilva/cep-promise that referenced this issue Mar 8, 2018

fix(typings): fix type for angular 5
in issue #112, we saw the problem with angular 5.

in my tests, i install angular 5 and rollback the definition without workarround
solved for default function Microsoft/TypeScript#5073

and this back work, i think this worked because typescript can have a fix in recent version.

ljani added a commit to ljani/tslint-webpack-plugin that referenced this issue Mar 21, 2018

Prefer a namespace over declare module
Apparently import * as alias syntax does not work with export = function:
Microsoft/TypeScript#5073

jrparish added a commit to jrparish/tslint-webpack-plugin that referenced this issue Mar 21, 2018

Typings: Prefer a namespace over declare module (#13)
* Prefer a namespace over declare module

Apparently import * as alias syntax does not work with export = function:
Microsoft/TypeScript#5073

* TslintOptions already includes format
@unional

This comment has been minimized.

Show comment
Hide comment
@unional

unional Mar 24, 2018

Contributor

With esModuleInterop available, you should not need this hack anymore.
You should be able to do:

import x from 'cjs'

// instead of 
import * as x from 'cjs'
Contributor

unional commented Mar 24, 2018

With esModuleInterop available, you should not need this hack anymore.
You should be able to do:

import x from 'cjs'

// instead of 
import * as x from 'cjs'
@aodinok

This comment has been minimized.

Show comment
Hide comment
@aodinok

aodinok Apr 2, 2018

Thanks! esModuleInterop works like a sharm!

aodinok commented Apr 2, 2018

Thanks! esModuleInterop works like a sharm!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.