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

Fix error TS2497 on import * as X from 'statsd-client': #10291

Merged

Conversation

garthk
Copy link
Contributor

@garthk garthk commented Jul 26, 2016

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.

Pinging @peterkooijmans and @horiuchi for review.

@dt-bot
Copy link
Member

dt-bot commented Jul 26, 2016

statsd-client/statsd-client.d.ts

to author (@peterkooijmans). Could you review this PR?
👍 or 👎?

Checklist

  • pass the Travis CI test?

@garthk
Copy link
Contributor Author

garthk commented Jul 26, 2016

Double-checked with the project in which I tripped over the problem:

$ typings install --global dt~statsd-client
$ tsc -p tsconfig.build.json
src/lib/util/statsd.ts(1,31): error TS2497: Module '"statsd-client"' resolves to a non-module entity and cannot be imported using this construct.
$ typings install --global 'github:garthk/DefinitelyTyped/statsd-client/statsd-client.d.ts#fix-statsd-client-import-asterisk'
$ tsc -p tsconfig.build.json
$ echo $?
0

@garthk
Copy link
Contributor Author

garthk commented Jul 26, 2016

I think it's safe to claim:

  • pass the Travis CI test?

UPDATE: aah, I see the note in the contribution guide, now. Closed and re-opened to kick Travis off…

UPDATE: fine. I'll make some change to the commit message and git push -f, then.

@garthk garthk closed this Jul 26, 2016
@garthk garthk reopened this Jul 26, 2016
@garthk garthk force-pushed the fix-statsd-client-import-asterisk branch from d077714 to 4810f1b Compare July 26, 2016 03:17
@horiuchi
Copy link
Member

@garthk Thank you for your contribution.
Looks good to me about this changing.

And please remove the statsd-client-import-asterisk-tests.ts.tscparams file.

@yuit yuit added the Revision needed This PR needs code changes before it can be merged. label Jul 26, 2016
@garthk
Copy link
Contributor Author

garthk commented Aug 8, 2016

@horiuchi, npm test fails without the statsd-client-import-asterisk-tests.ts.tscparams file because statsd-client-import-asterisk-tests.ts requires ES6 module style and targets. Please advise if there is some other way to test import * without the tscparams file.

statsd-client/statsd-client-import-asterisk-tests.ts(1,31): error TS2307: Cannot find module 'statsd-client'.

UPDATE: /// <reference path="statsd-client.d.ts" /> FTW

@garthk garthk force-pushed the fix-statsd-client-import-asterisk branch from 4810f1b to b0b5964 Compare August 9, 2016 00:28
@garthk
Copy link
Contributor Author

garthk commented Aug 9, 2016

Rebased on current master.

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 garthk force-pushed the fix-statsd-client-import-asterisk branch from b0b5964 to f7763de Compare August 9, 2016 06:15
@garthk
Copy link
Contributor Author

garthk commented Aug 9, 2016

@yuit, I've removed the file as @horiuchi requested and rebased on `master. I trust we're good to go.

@horiuchi
Copy link
Member

horiuchi commented Aug 9, 2016

@garthk Thanks!

@horiuchi horiuchi merged commit 8d9277b into DefinitelyTyped:master Aug 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Revision needed This PR needs code changes before it can be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants