denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
94.05k stars 5.23k forks source link

npm: automatic acquisition of `@types` package for type checking #20455

Closed nayeemrmn closed 4 months ago

nayeemrmn commented 1 year ago

This was already on the roadmap but with no issue.

You can usually work around this with // @deno-types="npm:@types/<package>" but that's not always an option. E.g. for "jsxImportSource": "npm:react" in your compiler options, there's no import to add the directive to. So it's a big obstacle for react-jsx users.

dsherret commented 1 year ago

I don't think we should do this.

  1. It's magical—the types package version isn't specified.
  2. It's extremely common for the version of a @types/<package-name> package to not align with the actual package version.
  3. Figuring out if we need to do automatic type acquisition will make npm resolution slower as we need to analyze the package for if it has types (which can only be done once the tarball has been downloaded and extracted), check if there's a @types/<package-name> for that version or approxiamte version (which could be wrong), and then finally fill in that association to deno_graph (or alternatively we could require downloading and extracting all top level packages before we complete the initial deno_graph resolution).
KyleJune commented 1 year ago

Not being able to use npm specifiers for jsxImportSource makes using react pretty painful in Deno. Anyone that wants to use it basically has to use esm.sh still because of this issue. See https://github.com/denoland/deno/issues/18203 for an example.

The current hack to get react types is pretty ugly and not obvious, the only reason I know about it is from finding that issue linked above. I don't have this problem with esm.sh and thought the point of npm specifiers was so that we wouldn't need esm.sh.

// See: https://github.com/denoland/deno/issues/16653
import type {} from "npm:@types/react@18";
import type {} from "npm:@types/react-dom@18";

The other option of adding a comment is a bit painful too. I don't think you need to do anything like that to get types for your npm dependencies in node.

// @deno-types="npm:@types/<package>"

It would be great if we could just import react and get the types we would expect to get if we did the same in node.

dsherret commented 1 year ago

@KyleJune I think we need a different solution for that then this. For example, this won't help if there is a version mismatch. Let's discuss in the other issue.

oscarotero commented 1 year ago

I think the main problem is not if the option is ugly or not, but there's no way to specify this globally. Maybe an option in deno.json to assign types to any module? Something like:

{
  "compilerOptions": {
    "jsx": "react-jsx",
    "jsxImportSource": "npm:react",
  }, 
  "types": {
    "npm:react": "npm:@types/react@18"
  }
}
dsherret commented 1 year ago

@oscarotero yes, see https://github.com/denoland/deno/issues/18203#issuecomment-1714879500 and below

sant123 commented 4 months ago

@dsherret should this work for any package right? I'm testing with mssql but I still need to specify the @deno-types comment.

With comment: image

Without comment: image

dsherret commented 4 months ago

Sorry, this issue shouldn't have been closed as part of jsxImportSourceTypes support. That said, I'm pretty sure this issue won't ever be done for the reasons I outlined above and so I'm going to close this as not planned.

KyleJune commented 2 weeks ago

Sorry, this issue shouldn't have been closed as part of jsxImportSourceTypes support. That said, I'm pretty sure this issue won't ever be done for the reasons I outlined above and so I'm going to close this as not planned.

Is there a way or a plan for a way to specify types for packages like the mssql package referenced in a previous comment other than adding a deno types comment everywhere you import it? I believe in node you can get it to acquire types by installing the @types/mssql package which adds an entry to your package.json file. Will that work in deno? If so, do we need a package.json if an npm package we depend on needs types or is there another way to get them in the deno.json file?

Edit: The best workaround I know of currently is adding .d.ts files for any npm packages missing types. For example, a react.d.ts file like this:

declare module "react" {
  // @ts-types="@types/react"
  import React from "npm:react@18";
  export = React;
}