TypeScript icon indicating copy to clipboard operation
TypeScript copied to clipboard

Disable type checking for node_modules entirely

Open gempir opened this issue 5 years ago • 65 comments
trafficstars

Search Terms

skipLibCheck node_modules ignore library exclude

Suggestion

Either a new option or the existing option skipLibCheck should be able to disable type checking for node_modules.

Use Cases

A library author might have a faulty import or might only provide typescript files for his library and not transpiled code. As a user of this library I want to be able to disable type checking for a library (or all) because I trust this library has been tested enough in other ways.

Another issue where I found a library shipping ts files and causing errors for the user: https://github.com/microsoft/TypeScript/issues/15363

The response at the end is very relevant

I understand that libraries shouldn't publish *.ts files, but sometimes, they just do, and we can't control every other library easily. I think typescript should not try to apply this kind of setting (here noImplicitAny) to the node_modules simply because it is meant to be applied to the user code being compiled, not the external dependencies.

Originally posted by @victornoel in https://github.com/microsoft/TypeScript/issues/15363#issuecomment-309459043

Examples

I had the case that a library came with a dist/ folder and almost all types were perfectly arranged in .d.ts files. But one file was making an import from ../index.d.ts(which seems to be for development purposes, the lib main file is dist/index.d.ts) which in turn then imported src/ClassA.ts

The library author used a different tsconfig and didn't use strict type checking, but I do and that's why the type check fails for me, because now the typescript source files of the library author are type checked.

Checklist

My suggestion meets these guidelines (in case of a new option):

  • [x] This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • [x] This wouldn't change the runtime behavior of existing JavaScript code
  • [x] This could be implemented without emitting different JS based on the types of the expressions
  • [x] This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • [x] This feature would agree with the rest of TypeScript's Design Goals.

gempir avatar Sep 08 '20 06:09 gempir

As a user of this library I want to be able to disable type checking for a library (or all) because I trust this library has been tested enough in other ways.

Okay, this is one of the reasons you might use skipLibCheck.

I had the case that a library came with a dist/ folder and almost all types were perfectly arranged in .d.ts files. But one file was making an import from ../index.d.ts(which seems to be for development purposes, the lib main file is dist/index.d.ts) which in turn then imported src/ClassA.ts

This is the opposite of the scenario you're describing. The library author has shipped incoherent types, and who's to say what the correct interpretation of that is? It is good that TypeScript errors here, even if it is an overall terrible user experience.


"Disabling type-checking" can mean a lot of things. Having the package come in as any is one possibility. If that's what you're looking for, then you can add a single global file with declare module "incorrectly-authored-library"; until the package ships correct types. Otherwise, I'm not sure what you might have in mind.

DanielRosenwasser avatar Sep 09 '20 19:09 DanielRosenwasser

I want to mention after reading this a second time: shipping .ts files is generally nice when using --declarationMaps but I don't know how to best address the issue of different settings between your project and the library's project. Figuring out good prescriptive advice for doing that might be one strategy here.

DanielRosenwasser avatar Sep 09 '20 19:09 DanielRosenwasser

Yeah there are lots of good reasons to actually have the sources of a library available, I like to explore them in my IDE sometimes or maybe while debugging etc.

But I don't actually want to have them checked by the typescript compiler, hence the skipLibCheck. It's just so confusing to me why the skipLibCheck is only intended for typing files and not for .ts files

gempir avatar Sep 09 '20 19:09 gempir

This scenario is going to be completely busted for a lot of reasons. It seems like the option one would really want, if they found themselves stuck in it, would be to say that .ts files under some given paths should be treated as if they were .d.ts files (don't emit, don't typecheck under SLC, don't use to compute rootDir, etc) but have their implementations used for inference. That said, this is just critically a misconfiguration and it's possibly counterproductive to do gymnastics to make it work.

RyanCavanaugh avatar Sep 09 '20 23:09 RyanCavanaugh

I think .ts files imported by .d.ts files should be treated the same way, so if skipLibCheck is enabled it should ignore them.

If you are importing .ts files from actual source files like dist/main.js importing ../src/ClassB.ts then i just expect it to break. That shouldn't be ignored.

gempir avatar Sep 10 '20 06:09 gempir

Could there be a way to just silence errors from certain paths?

I don't want to make a whole import from node_modules type any as 99% of the types are fine and are useful for code completion, but I don't want to hear about the 1% of types that don't adhere to my tsconfig when I can't do anything about them (aside from fork the whole module for the sake of a couple of errors).

Noting that a single type check fail would break CI unless I silence it some how.

shadow-light avatar Aug 19 '21 12:08 shadow-light

(I tried @ts-ignore above the import with the issue, but the errors are with certain files in that imported module, so obviously didn't work)

shadow-light avatar Aug 19 '21 12:08 shadow-light

I am having a very similar issue with a node_module that is included locally in package.json with a path: file:.... there are type errors happening inside of that library and the compiler does not ignore them despite having "skipLibCheck": true in the config.

NonkelDaniel avatar Aug 20 '21 09:08 NonkelDaniel

I had the case that a library came with a dist/ folder and almost all types were perfectly arranged in .d.ts files. But one file was making an import from ../index.d.ts(which seems to be for development purposes, the lib main file is dist/index.d.ts) which in turn then imported src/ClassA.ts

The library author used a different tsconfig and didn't use strict type checking, but I do and that's why the type check fails for me, because now the typescript source files of the library author are type checked.

Just ran into this exact scenario trying to type check my build scripts. Seems like not many people do that so some build-related modules have issues that go unnoticed.

shadow-light avatar Aug 20 '21 11:08 shadow-light

Here's an example of a project that seems to be doing something benign, simply importing an interface for the sake of type checking. But the import path points to distributed typescript source files rather than the compiled JS. And while there aren't any issues with the types of that module, because my project has stricter type check settings it causes issues.

https://github.com/underfin/vite-plugin-vue2/pull/125/files

shadow-light avatar Aug 20 '21 12:08 shadow-light

I'm having this problem with the lib tst-reflect/tst-reflect-transformer: "TS6133 declared but value never read" I want that strict checking on our project but would love if it was not enforced on specific libraries I hand pick.

Compiler opts, ran with ttypescript's ttsc:

  "compilerOptions": {
    "allowSyntheticDefaultImports": true,
    "alwaysStrict": true,
    "esModuleInterop": true,
    "module": "commonjs",
    "noEmitOnError": true,
    "noImplicitAny": true,
    "noImplicitReturns": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "sourceMap": true,
    "strict": true,
    "strictFunctionTypes": false,
    "useUnknownInCatchVariables": false,
    "noImplicitThis": false,
    "target": "ES6",
    "lib": [
      "es6",
      "dom",
      "es2015.promise",
      "es2016.array.include"
    ],
    "outDir": "build",
    "plugins": [
      { "transform": "tst-reflect-transformer" }
    ]
  },

nitram-work avatar Nov 23 '21 12:11 nitram-work

We have been forced to disable strict: true on all our repos.
Third-party libraries ship only .ts files. The package.json has entry point as index.ts. After a year of requests and pressure, no progress has been made convincing other companies to make changes to what they publish.

The code compiles properly, and works at runtime. However, we wish to have strict: true enabled in our builds. However this is not possible. Required packages publish only raw .ts files, which do not meet strict: true, preventing automated enforcement of standards within code that we do have control over.

Being unable to enable any strict typing, solely because we consume third-party packages that don't meet the same strictness, and of which we have no leverage for change:

  • use of any,
  • function type mismatches (../../node_modules/@***/***/src/sdk/*****Api.ts:302:9 - error TS2322: Type '(value?: void | PromiseLike<void>) => void' is not assignable to type '(value?: unknown) => void'
  • ../../node_modules/@***/***/src/sdk/*****Api.ts:21:16 - error TS2532: Object is possibly 'undefined'

We have a manual process that is sometimes applied:

  • check out a PR,
  • manually perform a tsc --noEmit --project tsconfig-strict.json,
  • filter out hundreds of errors from node_modules/ .ts file only libraries
  • find errors in code we own Which is error prone, and allows typing problems in our code to slip through.

I had suggested something similar to this : https://github.com/microsoft/TypeScript/issues/44205. But it was rejected as "configuration issue".

icywolfy avatar Dec 08 '21 20:12 icywolfy

Can we please get a flag to suppress any error messages from files originating from node_modules? Doesn't matter if there are 1 million errors if they are in node_modules, just ignore them, don't display them, and exit with code 0. The only time when we should exit with code 1 is when there are errors anywhere else other than node_modules

skipLibCheck is NOT a solution, it's going to disabled ALL declaration files checks, including YOURS!

An example of unwanted error:

node_modules/react-native-ui-lib/typings/generatedTypes/colors.d.ts:38:5 - error TS2300: Duplicate identifier 'grey10'.

38     grey10: string;
       ~~~~~~

node_modules/react-native-ui-lib/typings/generatedTypes/colors.d.ts:39:5 - error TS2300: Duplicate identifier 'grey20'.

39     grey20: string;
       ~~~~~~

node_modules/react-native-ui-lib/typings/generatedTypes/colors.d.ts:40:5 - error TS2300: Duplicate identifier 'grey30'.

40     grey30: string;
       ~~~~~~

node_modules/react-native-ui-lib/typings/generatedTypes/colors.d.ts:41:5 - error TS2300: Duplicate identifier 'grey40'.

41     grey40: string;
       ~~~~~~

node_modules/react-native-ui-lib/typings/generatedTypes/colors.d.ts:42:5 - error TS2300: Duplicate identifier 'grey50'.

42     grey50: string;
       ~~~~~~

node_modules/react-native-ui-lib/typings/generatedTypes/colors.d.ts:43:5 - error TS2300: Duplicate identifier 'grey60'.

43     grey60: string;
       ~~~~~~

node_modules/react-native-ui-lib/typings/generatedTypes/colors.d.ts:44:5 - error TS2300: Duplicate identifier 'grey70'.

44     grey70: string;
       ~~~~~~

node_modules/react-native-ui-lib/typings/generatedTypes/colors.d.ts:45:5 - error TS2300: Duplicate identifier 'grey80'.

45     grey80: string;
       ~~~~~~

node_modules/react-native-ui-lib/typings/generatedTypes/colors.d.ts:46:5 - error TS2300: Duplicate identifier 'grey10'.

46     grey10: string;
       ~~~~~~

node_modules/react-native-ui-lib/typings/generatedTypes/colors.d.ts:47:5 - error TS2300: Duplicate identifier 'grey20'.

47     grey20: string;
       ~~~~~~

node_modules/react-native-ui-lib/typings/generatedTypes/colors.d.ts:48:5 - error TS2300: Duplicate identifier 'grey30'.

48     grey30: string;
       ~~~~~~

node_modules/react-native-ui-lib/typings/generatedTypes/colors.d.ts:49:5 - error TS2300: Duplicate identifier 'grey40'.

49     grey40: string;
       ~~~~~~

node_modules/react-native-ui-lib/typings/generatedTypes/colors.d.ts:50:5 - error TS2300: Duplicate identifier 'grey50'.

50     grey50: string;
       ~~~~~~

node_modules/react-native-ui-lib/typings/generatedTypes/colors.d.ts:51:5 - error TS2300: Duplicate identifier 'grey60'.

51     grey60: string;
       ~~~~~~

node_modules/react-native-ui-lib/typings/generatedTypes/colors.d.ts:52:5 - error TS2300: Duplicate identifier 'grey70'.

52     grey70: string;
       ~~~~~~

node_modules/react-native-ui-lib/typings/generatedTypes/colors.d.ts:53:5 - error TS2300: Duplicate identifier 'grey80'.

53     grey80: string;
       ~~~~~~

Okay, so? I don't really mind that there are type errors on the libraries we use, I only mind that there are type errors on the codes that we wrote anywhere else other than node_modules.

I have 121 errors all of which are from node_modules.

Can someone explain what's the reasoning why we would want to be blocked by errors coming from node_modules? I want to implement a tsc --noEmit on a github check so that every PR will be checked before we event want to review the changes. When the github check fails, we should not be able to merge the PR because the check ensures code quality.

aprilmintacpineda avatar Jan 18 '22 12:01 aprilmintacpineda

My project is fully in js but I do use IntelliSense with strict typing so that I get early warnings when mistakes have been made. But since both checkJs and strict are enabled, I now get a ton of errors from node_modules.

Normally this wouldn't be a huge issue, but I have also enabled Project Diagnostics, which makes errors from all files show up, even if they're not open. Which makes it hard to see a warning whenever a file contains a mistake. If only a single file has an error, it immediately sticks out like a sore thumb, whereas if you permanently have a bunch of files and folders that are red, not so much. Not only that, the Problems view now has a huge list of errors that makes it impossible to find errors caused by me, unless I filter them first.

I also wish to include type checking in CI, but this makes that impossible without writing some sort of plugin to filter all unnecessary errors.

As far as I'm aware, there's a few options to fix this:

  • Disable checkJs and place @ts-check at the top of all my files. This is prone to mistakes, but a linter rule might be able to prevent that.
  • Add @ts-nocheck for all files with errors in node_modules. But it would have to be done with some sort of script to prevent overwrites when updating modules.
  • Disable Project Diagnostics, but this means I won't get warnings from files that are not open. And making a change to a file doesn't necessarily mean new errors will occur in that file only.

But neither of these seem like a satisfying solution to me. If it were possible to add a list of paths to the tsconfig/jsconfig similar to the include and exclude properties, it would be possible to just ignore "node_modules/**/*.js" or "node_modules/*" entirely. This would basically function the same as placing @ts-nocheck at the top of each file. This seems like a nice solution to me, with the added benefit that files other than those in the node_modules directory can be configured to be ignored as well.

jespertheend avatar Jan 26 '22 17:01 jespertheend

The irony is that this issue is not about no-name packages.

Microsoft authentication library for js does throw an error during build under strict ts mode: Type 'string | undefined' is not assignable to type 'string'

Btw, check out their average code quality) Declared return type - 'Account', actual return type - 'Account | null'.

snowinmars avatar Jul 28 '22 08:07 snowinmars

@snowinmars That library has a correct typings field so you shouldn't be seeing that error, can you clarify how your project is configured? https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-core/package.json#L24

An external library not having strictNullChecks on is not anyone's defect

RyanCavanaugh avatar Jul 28 '22 17:07 RyanCavanaugh

@snowinmars That library has a correct typings field so you shouldn't be seeing that error, can you clarify how your project is configured? https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-core/package.json#L24

An external library not having strictNullChecks on is not anyone's defect

Msal can be imported in two ways:

  • you can import prebuild items from lib-* folders - and that will not break your build
  • you can import sources from src folder itself to build it by yourself - and that will release the chaos

It's about import { AuthError } from 'msal' vs import { AuthError } from 'msal/src/error/AuthError'

Technically, one can say, that that's what prebuilds are for. But I would not agree: if developers doesn't want the source to be linked, they do not include it to npm package. So, it's a legal way to use a library, that just doesn't match with another's developer way.

If typescript compiler could solve this kind of issues, that'd be cool.

P.S. There is a @azure/msal-browser package with MUST better typings.

snowinmars avatar Jul 29 '22 18:07 snowinmars

If the MSAL authors are telling you to import the TS sources directly from their packages, please tell me where so I can go tell them to fix their docs. This isn't a supported scenario and there's no end of problems this will cause.

RyanCavanaugh avatar Jul 29 '22 20:07 RyanCavanaugh

There is still no solution for “skipping node modules” typings? Because the package I use don't have correct typing on its own…

fatihaziz avatar Oct 25 '22 19:10 fatihaziz

Just ran into this myself. I'd really like to add a pipeline stage that runs a simple type check on our repo, but this is blocking that from happening. Is there really no workaround?

bmarden avatar Nov 10 '22 19:11 bmarden

I can't imagine the amount of packages that have this exact problem which end up just not using strict mode and introduce iffy code making everything worst downstream.

nitram-work avatar Nov 10 '22 21:11 nitram-work

Having this issue myself as well :/

hiddentao avatar Nov 19 '22 23:11 hiddentao

Same here.

cerskeenboy avatar Dec 20 '22 10:12 cerskeenboy

same here

karimch27 avatar Jan 05 '23 10:01 karimch27

Can people post "Here's how I ended up importing .ts files from node_modules" or "Here are the errors in a .d.ts in node_modules I had" instead of "same" ?

RyanCavanaugh avatar Jan 05 '23 17:01 RyanCavanaugh

Mine is fixed by:

  • create a fork of the problematic package
  • fix the typing by my own
  • npm pack
  • install the tar using npm install package-v1.tar
  • then calling out the author, that their typing broken,
  • create pull requests
  • fixed.

fatihaziz avatar Jan 05 '23 17:01 fatihaziz

@RyanCavanaugh What's the difference between importing JS with typing files and importing straight TS? I.e. what makes it difficult to support both?

I'm guessing it is to do with other TS features that are compiled to plain JS that aren't necessarily in the typing files?

These days every module has its own opinion about what level of JS to support, and an app may want to target an older version than a module wants to. I wonder if it would be better to publish plain TS and let apps decide what JS version to compile down to for both the app code and module code.

There's probably numerous challenges and performance issues with that, but just a thought...

shadow-light avatar Jan 06 '23 01:01 shadow-light

There's no such thing as "importing straight TS" from another module which has a different configuration from yours (and, as you point out, different people having different configurations is very common). There are at least a dozen different compiler flags that might change the interpretation of a line of implementation code. The process of making .d.ts files takes away most of those such that any two projects can communicate in a sort of "common language", but if you take the .ts file itself then its true type behavior is not really known without also knowing its configuration.

We "could" support TS importing files and looking up their true configuration and reinterpreting the code differently in different regions of the code, but this is a huge mess with really bad performance and predictability characteristics. So all of this is to what end? The package can just publish its .d.ts and, under a modern module ecosystem, a .js file targeting a modern JS language variant, which you can bundle and downlevel to whatever target. Bundlers already do a great job of that, including features TS doesn't have like tree-shaking. We don't need to reimplement the wheel.

RyanCavanaugh avatar Jan 06 '23 05:01 RyanCavanaugh

Can people post "Here's how I ended up importing .ts files from node_modules" or "Here are the errors in a .d.ts in node_modules I had" instead of "same" ?

Transformer to add reflection features to the compiler: https://github.com/Hookyns/tst-reflect https://www.npmjs.com/package/tst-reflect-transformer

nitram-work avatar Jan 06 '23 09:01 nitram-work

Can people post "Here's how I ended up importing .ts files from node_modules" or "Here are the errors in a .d.ts in node_modules I had" instead of "same" ?

Transformer to add reflection features to the compiler: https://github.com/Hookyns/tst-reflect https://www.npmjs.com/package/tst-reflect-transformer

🙏🙏🙏 Really appreciate you share this

fatihaziz avatar Jan 06 '23 09:01 fatihaziz