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

Better handling of untyped node module imports #11106

Closed
bterlson opened this issue Sep 23, 2016 · 15 comments
Closed

Better handling of untyped node module imports #11106

bterlson opened this issue Sep 23, 2016 · 15 comments
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@bterlson
Copy link
Member

Consider importing an untyped module, eg. exceljs:

import exceljs = require('exceljs');

This results in an error that the module exceljs is not found. There is at least two ways to fix this:

  1. Setting allowJs: true and maxNodeModulesJsDepth: 1
  2. Declare an ambient module in a separate file

I think it would be better if instead we update module resolution semantics such that when we find an appropriate package.json without a typings entry, we store it away. Then if module resolution fails (ie. we also can't find an @types/exceljs), we go back and import the untyped package.json as a JS import. Thoughts?

You might argue that this should only be allowed with --allowJs, but it might also make sense to just always use this behavior when using node resolution rules as you may still not want to allow JS in your own project while still allowing your dependencies to be authored in JS without typings.

@mhegazy mhegazy added Suggestion An idea for TypeScript Committed The team has roadmapped this issue labels Sep 23, 2016
@mhegazy mhegazy added this to the TypeScript 2.1 milestone Sep 23, 2016
@mhegazy mhegazy assigned ghost Sep 23, 2016
@aluanhaddad
Copy link
Contributor

Is there anyway we can solve this in a non npm specific manner? I like the concept but tying it to node_modules/import_name and node_modules/@types/import_name is not something I would like to see.

@dmitriid
Copy link

Also, please, please, please improve documentation on "ambient modules". I've read the section seven times now, I still have no idea what the ambient modules are, how to define them and where to put the specs so that they are picked up by TS. Espec

It would be awesome, if there was a small section "you want to import module X. you create a file X.d.ts here. Then you do this and that and this"

@ghost ghost mentioned this issue Oct 27, 2016
@ghost ghost closed this as completed in #11889 Oct 27, 2016
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Oct 27, 2016
@reverie
Copy link

reverie commented Nov 16, 2016

I followed a chain of four closed tickets to get here. Glad to see the issue was addressed. Could someone please point to docs for this feature? How do I use it? For my sake, and for everyone else who may end up here. :)

@mhegazy
Copy link
Contributor

mhegazy commented Nov 16, 2016

This is part of [email protected] release, so for now you will need npm install typescript@next.

Now having an import like import foo from "Foo" the compiler will check if there is a package Foo, e.g. node_modules\Foo\index.js if one is available, even if it does not have a .d.ts file along with it, no error will be reported. the type of the imports will be any.

if you use --noImplicitAny, you would get these import flagged as implicit anys.

hope that helps.

@reverie
Copy link

reverie commented Nov 16, 2016

That helps a lot, thank you. It seems like for now (v2.0.10) we can also use declare module "Foo";?

@mhegazy
Copy link
Contributor

mhegazy commented Nov 16, 2016

yes. that is Shorthand ambient module declarations

@radicaled
Copy link

Now having an import like import foo from "Foo" the compiler will check if there is a package Foo, e.g. node_modules\Foo\index.js if one is available, even if it does not have a .d.ts file along with it, no error will be reported. the type of the imports will be any.

Does this mean that if you've got a node module that doesn't use index.js but instead relies on a main field in package.json that this feature doesn't work?

@ghost
Copy link

ghost commented Dec 3, 2016

@radicaled We check for that too.

@lvpro
Copy link

lvpro commented May 13, 2017

Now having an import like import foo from "Foo" the compiler will check if there is a package Foo, e.g. node_modules\Foo\index.js if one is available, even if it does not have a .d.ts file along with it, no error will be reported. the type of the imports will be any.

And what about relatively referenced JS modules? import Blah from './blah.js';

In this case, Blah ends up as "type Blah" and TS tries its best to infer the shape of the exported object from blah.js, which in my case, was wrong (it only identified some of the properties on the object - others get errors of property key does not exist on value of type Blah). Hence, Blah is NOT of type any! I have to do (Blah as any).method() to get it to work.

I would think many would be hitting this issue mixing TS and JS. We're using an ejected create-react-app with TS support added to the webpack config via ts-loader. I expected it to just work without fuss, sadly not the case.

Longer explanation with code sample: http://stackoverflow.com/questions/43954320/es6-import-of-relative-path-js-module-type-not-fully-inferred-how-to-declare

@ghost
Copy link

ghost commented May 13, 2017

Hi @lvpro, if the file does not come from node_modules it is presumed to be your own code, so TypeScript will infer types. As I see it you have 2 options:

  • Quick fix: disable all inference for that file by adding blah.d.ts containing:
declare const blah: any;
export default blah;
  • Add enough info for TS to infer the existence of those methods (if not the parameter/return types). You could add this to the constructor:
/** @type {Function} */
this.get = this.post = this.put = this.delete = undefined;

And you will now get these methods in autocomplete, although you won't get signature help.

@ghost
Copy link

ghost commented May 13, 2017

The first suggestion would go in its own file, not in a declare module.

@lvpro
Copy link

lvpro commented May 13, 2017

Does quick fix 1 only work if blah.d.ts is in the same location as blah.js? It does work, but my team may not want .d.ts files polluting their code directories (we're slowly using Typescript on new areas of the system, 95% of it is an ES6 codebase).

@mhegazy
Copy link
Contributor

mhegazy commented May 13, 2017

Does quick fix 1 only work if blah.d.ts is in the same location as blah.js?

If you import the module with its name (i.e. import d from "foo";) and not with a relative path (i.e. import d from "./foo";) then you can either 1. put the .d.ts next to the .js, 2. put it somewhere else, and add a path mapping, or 3. add it to a folder, e.g. ./types/foo/index.d.ts and add ./types to your typeRoots in the tsconfig.json.

if it is a relative import, then you need to put the .d.ts next to the .js

@lvpro
Copy link

lvpro commented May 13, 2017

So in instances where one imports relatively (this would be especially common in a create-react-app codebase), it seems these are the options;

  1. Hope the TS compiler can infer your JS right
  2. If it doesn't, you have these options:
  • (blah as any).method() on every use of the import
  • add a new variable to your .ts such as const blahAny: any = blah;
  • add a .d.ts to the same location as the .js, which may not be desirable or appreciated by the owners of that location.

I wish we could opt-in to having the same behavior as modules within node_modules, wherein if a typing is not found, the module could default to type any. I saw someone suggest import blah: any from './blah.js', which would be clean and simple syntax specifying the programmer's intent.

@ghost
Copy link

ghost commented May 16, 2017

One thing you could do is use an untyped re-export:

// ts-module.ts
import foo from "./js-module";
export default foo as any;

Then import from ts-module.ts instead.
You could also use this to give a better type than any without touching the JS.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants