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

Use dtsGen to generate declaration files for imported modules with no declarations #25746

Closed
dannycochran opened this Issue Jul 17, 2018 · 23 comments

Comments

Projects
None yet
8 participants
@dannycochran
Copy link

dannycochran commented Jul 17, 2018

Search Terms

dts-gen
third party modules
fallback
typings
modules without typings

Suggestion

One of the biggest concerns I've heard from new users using TypeScript is dealing with libraries that have no typings provided (neither in the library itself nor in DefinitelyTyped). In a strict TS environment or with noImplicitAny: true, TypeScript will complain about a library you import that has no associated typings definition.

Your options at this point are to provide your own typings file in a local typings folder, or use dts-gen to generate one for you.

Personally, I find this to be a useful exercise but newer users get tripped up on it and frequently cite it as one of the harder parts about TS adoption.

I had considered creating a library (similar to https://github.com/tamayika/types-local) that would use dts-gen to automatically put index.d.ts files inside of libraries within your node_modules folder that have no associated typings, provided you've imported the library in your code. This would probably end up being gross and an npm anti-pattern.

Instead, it would be nice if the compiler could do this for us. If you have imported code w/o typings, the TS compiler could use dts-gen to generate typings (which obviously are imperfect) which would allow folks to proceed without worrying immediately about whether the library has typings.

Maybe a flag called dtsGenFallback under compilerOptions could do this?

Use Cases

Described above

Examples

Described above

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript / JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. new expression-level syntax)
@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Jul 18, 2018

I do not think the compiler is the right place for this feature. the language service, however, is. This is a feature that you do not do on every build, nor should it. it is rather one that you do once when you first import a package.

@mhegazy mhegazy changed the title Add "dtsGenFallback" to compilerOptions Use dtsGen to generate declaration files for imported modules with no declarations Jul 18, 2018

@mhegazy mhegazy added this to the TypeScript 3.1 milestone Jul 18, 2018

@dannycochran

This comment has been minimized.

Copy link
Author

dannycochran commented Jul 18, 2018

@mhegazy thank you for the quick response and triaging!

That makes sense that it would be under the language service. I think the only thing you'd need to check for on build is if the package updated, as an updated package may now have typings associated with it, or the output from dts-gen could be different.

To clarify though, how would you imagine users activating this feature?

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Jul 18, 2018

I think the only thing you'd need to check for on build is if the package updated,

that is a general problem that we have today as well with the quick fix to install @types packages. Today we install the latest @types package regardless. ideally we would be more cognizant of the version and install the right version. Then the next problem is detecting version mismatches and warning the user about them, giving them the option to update. I think this feature would follow the same pattern.

how would you imagine users activating this feature?

If a module has no declaration file in it, no matching @types package in the registry, and no ambient module declaration, the user will get a Suggestion diagnostic (three little dots under the name in VSCode), along with a light pulp to create the declaration file for the missing package.

@dannycochran

This comment has been minimized.

Copy link
Author

dannycochran commented Jul 18, 2018

If a module has no declaration file in it, no matching @types package in the registry, and no ambient module declaration, the user will get a Suggestion diagnostic (three little dots under the name in VSCode), along with a light pulp to create the declaration file for the missing package.

Gotcha. I like that as it forces the user to understand what's happening, and that the typings are going to be very lax (dts-gen tends to do a lot of any typing).

Where would the declaration file go? I suppose you could put it into node_modules/<untypedPackage>/index.d.ts, but then it doesn't get checked in.

If you put it into the app directory, you'd probably want to check if there's already a local typings folder for untyped packages or for augmented third party typings. Determining where that existing folder lives is probably non-obvious.. I suppose you could scan compilerOptions.typeRoots and look for a local typings folder?

Separately, if a user has no local typings folder, you'd have to create one for them and modify their compilerOptions.typeRoots. Not sure the language service can do that?

@mhegazy

This comment has been minimized.

Copy link
Contributor

mhegazy commented Jul 18, 2018

Where would the declaration file go? I suppose you could put it into node_modules//index.d.ts, but then it doesn't get checked in.

node_modules is not the best place. since that is not checked in your project. we will have to pick a name for a folder for these packages. I think we can use typeRoots as a way of inferring the location of types.

@dannycochran

This comment has been minimized.

Copy link
Author

dannycochran commented Sep 28, 2018

@andy-ms thanks for working on this! I noticed you marked the issue as fixed, but in the PR it says the functionality is not yet exposed to consumers? I just tried testing in 3.1 and didn't see a code fix lightbulb.

@andy-ms

This comment has been minimized.

Copy link
Member

andy-ms commented Oct 1, 2018

@dannycochran It's working for me in typescript@3.1 with the following setup:
node_modules/foo/index.js: exports.x = 0;
src/a.ts: import { x } from "foo";
tsconfig.json: { "compilerOptions": { "module": "commonjs" } }

Make sure you have your editor set up to use the latest TypeScript install. (vscode instructions)

@dannycochran

This comment has been minimized.

Copy link
Author

dannycochran commented Oct 10, 2018

hmm @andy-ms I tried that example locally and it does not work for me still. here's an example repo, if you run:

npm i && npm run build

you'll see the error.

@andy-ms

This comment has been minimized.

Copy link
Member

andy-ms commented Oct 11, 2018

Hi @dannycochran, this feature doesn't affect compilation. But if you open index.ts in an editor and click on 'foo' (which should be highlighted with the error), there should be a codefix that offers to generate the type.

@dannycochran

This comment has been minimized.

Copy link
Author

dannycochran commented Oct 15, 2018

@andy-ms I'm not seeing the codefix in VSCode (1.28.0) either, and the lightbulb setting is enabled in my global settings. VSCode is also pointed at my repository's version of TypeScript, 3.1.1.

There's one code fix available for converting to a namespace import, but that's not the right one.

autotypings1

@andy-ms

This comment has been minimized.

Copy link
Member

andy-ms commented Oct 16, 2018

@dannycochran In that image, it looks like 'foo' is at the root, rather than inside node_modules. In order for this feature to work we need to have resolved the module to a .js file, but it probably did not resolve at all. You can use --traceResolution to get information about that.

@dannycochran

This comment has been minimized.

Copy link
Author

dannycochran commented Oct 16, 2018

@andy-ms it's checked into the root, but it gets copied to node_modules on postinstall, so it ultimately lives at node_modules/foo/index.js -- I did that so it'd be easy to clone this repo and replicate, since node_modules are git ignored.

are you able to clone the repo I showed above and see the correct code fix after running npm install?

@andy-ms

This comment has been minimized.

Copy link
Member

andy-ms commented Oct 17, 2018

It works for me when I run npm install in your repo first. Not sure what's breaking. Could you hit ctrl-shift-p to open the command window and run TypeScript: Open TS Server log to see if an error might be happening?

@dannycochran

This comment has been minimized.

Copy link
Author

dannycochran commented Nov 8, 2018

@andy-ms

This comment has been minimized.

Copy link
Member

andy-ms commented Nov 9, 2018

It looks like you're encountering #27867. As of #28258 we should be producing better error info in that situation, so it would help if you installed typescript@next and tested with that.

@dannycochran

This comment has been minimized.

Copy link
Author

dannycochran commented Nov 9, 2018

Ran npm install typescript@next --save-dev (resolved to 3.2.0-dev.20181107), and pointed VSCode to use the local install from node_modules. Looks like I'm getting that same error (Expected isNewFile):

https://gist.github.com/dannycochran/87622db7624d9cadd41caa0dfa9f8d24

@arogg

This comment has been minimized.

Copy link

arogg commented Nov 10, 2018

I think a way to quickly augment typings from the DT repository would be more useful for the growth of TypeScript. I imagine just editing the typings from DT where all stuff surrounding pull requests and tests is abstracted away from the user and the user is guided through it automatically.
No one wants to clone the DT repo, read a bunch of documentation and create pull requests. I mean it has to happen, but the user does not want to associate with additional work, they want it to be easy and "fun". Also I want to use any typings I edit directly, without waiting for the pull request to get through.
I think because such a tooling does not exist, a lot of people don't bother contributing to DT and just create their typings locally (me too).
This hurts TypeScript MASSIVELY I think. But because you can't measure the detrimental effect, it goes completely unnoticed.
I want to make a feature request surrounding this because I am sure something is missing here, but I am uncertain how I would go about this, especially since I don't really have any idea on how to implement something like this. I just know that it would be better if not everyone was just creating typings for themselves (or using "any").
Anyone with me on this?

@andy-ms

This comment has been minimized.

Copy link
Member

andy-ms commented Nov 10, 2018

a way to quickly augment typings from the DT repository would be more useful

We already have this. You can, for example, augment any module with new exports, or add new members to its interfaces. https://www.typescriptlang.org/docs/handbook/declaration-merging.html

@arogg

This comment has been minimized.

Copy link

arogg commented Nov 10, 2018

a way to quickly augment typings from the DT repository would be more useful

We already have this. You can, for example, augment any module with new exports, or add new members to its interfaces. https://www.typescriptlang.org/docs/handbook/declaration-merging.html

There is a misunderstanding here. I'm talking about augmenting the DT repository, not just their types. (And in some cases declarations cannot be merged, btw.)

Just augmenting the DT types for personal use is OK, but only good in the short term. It does not benefit the community and you now must share that augmented typing between all your projects that use that package.

More specifically, I'm talking about the process of cloning the DT repo, reading the DT docs, augmenting the typings, maybe creating the missing type, adding the tests, pushing and creating the pull request and waiting until the PR is approved, so that you may then replace your manually copied/augmented DT typing with the new official @types packages. The last part is the most annoying for me but actually the whole process should be thought about.

I want that process highly integrated and streamlined.

I want to go to that DT type from within VS code and edit it, augment the DT tests and everything else is taken care of. This is just a rough idea of how it should be. I have not thought this out. But I think solutions will present themselves.

You want people to prefer adding to DT to just locally creating types for themselves. So you need to make the process fast and enjoyable. I also think this is something TypeScript can still benefit lots from. I very VERY often see inexact DT typings. It is not uncommon. I think this warrants additional attention for the issue.

Getting people to the point that they prefer adding to the DT repository, rather than creating local typings, should be the goal. I am sure DT is not tapping it's full potential. LOTS (and lots) of people still prefer just creating typings for themselves. Naturally, statistics for this do not exist, but I think it's self-explainatory :)

@dannycochran

This comment has been minimized.

Copy link
Author

dannycochran commented Nov 10, 2018

I want to make a feature request surrounding this because I am sure something is missing here, but I am uncertain how I would go about this

You can create a new issue with a feature request template:

https://github.com/Microsoft/TypeScript/issues/new?template=Feature_request.md

In general, what you're describing could be built by the TS team, or it could be built by a member of the TS community. I think it's a good idea, and it sounds like you're unsure about where to start. I'd suggest by starting a dedicated issue for the conversation, or you could join the TypeScript gitter channel and try vetting the idea there.

I'm not sure this issue is the right place to vet your idea, though. This feature is landed and merged to master, and in no way precludes the feature you're describing. I believe they would be complimentary.

@whitecolor

This comment has been minimized.

Copy link

whitecolor commented Nov 30, 2018

Currently contributing to DT is a crappy endeavor.

@dannycochran

This comment has been minimized.

Copy link
Author

dannycochran commented Dec 1, 2018

@andy-ms did you get a chance to look at the logs I posted here following your previous comment?

#25746 (comment)

@sandersn

This comment has been minimized.

Copy link
Member

sandersn commented Dec 14, 2018

@dannycochran Andy moved to the .NET team so now I'm taking a look at this codefix to see why it doesn't work. I'll take a look at your latest logs after I work through the earlier repros in this thread.

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