Repository navigation
Support Not applying strict* checks to node_modules / imported libraries.Β #44205
Description
Activity
MartinJohns commented
on May 21, 2021 ContributorMore actionsThis is very likely the same as #31035.
This is much more narrow in scope than #31035. Although, solving #31035 would indeed solve this issue.
The main differences:
- I propose no per directory tsconfig files, or ability to configure rules separately for different directories. Behaviour within the source tree is to remain as is.
- There is no ability to change configuration arbitrarily. The proposal is limited to strict mode checks only.
- There is no ability to configure "custom" strict mode checks, all strict mode checks are fully disabled when the referenced file is located within node_modules/.
The aim is that these limitations narrow the scope enough for the feature to be implementable.
A minimal proposed configuration would be a new compiler option:
--noStrictOnNodeModules. Which, while processing any file, if the referenced (not resolved if symlinked) file location, is in a node_modules directory, all strict mode reporting is not applied.Any additional configuration beyond this is out of scope.
As I am unaware of the internals of typescript, I omitted the following possible improvements from consideration:
--noStrictOnImports. Imported libraries, referenced by absolute path, have strict mode reporting disabled. This was disqualified as a file may be referenced by both absolute path, or by relative. In particular for monorepos; or linked packages.--disableStrictForPath=[globPattern]. This adds complexity if the glob pattern is in the project files to be compiled. It is useful for disabling strict mode for testing, via**/__tests__/**. However, should the glob pattern capture a file in the project to be compiled, the strict check is effectively meaningless, and encourages bad design. The complication of determining whether a glob affects a project file to compile is unknown. This idea was disqualified as encouraging bad-design, and duplicating the existing feature of//@ts-ignore.- addedDeclinedThe issue was declined as something which matches the TypeScript visionThe issue was declined as something which matches the TypeScript visionSuggestionAn idea for TypeScriptAn idea for TypeScript
on May 27, 2021 RyanCavanaugh commented
on May 27, 2021 MemberMore actionsThe problem I have with this, above and beyond the concerns/limitations described in #31035, is that this is ultimately (at least for now) a pure configuration error. You might be able to flag your way out of some of the problems here but there isn't a solution that would, for example, stop tsc from trying to emit
.jsfiles from these non-.d.tsinput files. The only correct path forward is to not have a.tsfile in yournode_modules.The most straightforward escape hatch for this misconfiguration is something more like an option to say "all files outside my
rootDirare fine but please treat them like.d.tsfiles"), but even that's a bit of a wreck in this case because you're going to pay a bunch of performance penalty every time doing inference on these files.If you have folders that are separate compilation units that aren't external code (i.e.
node_modules), project references already exist to enable the scenario of disparate compilation settings on separate parts of your project.I don't think we should do work to enable a scenario like this that's ultimately doomed to be unacceptably suboptimal.
Reacted by Reuben SteenekampRyan Cavanaugh (@RyanCavanaugh)
After 6 months of attempting to pressure External Company to do something about their provided packages, they have pushed back effectively saying : we publish working TypeScript packages; there is nothing to fix. Dozens of other companies consume our packages with no issue. This is your problem not ours.Is there -any- work-around, good-or-hack, that you can think of to allow us to enable
strict: truein our build system, and for developers, so that code-quality can be enforced, while ignoring errors in third-party code that we have no control over?Now, none of our projects can enable
strict: true. Doing so would break the build due to an increasing number of third-party libraries of which we have no influence over, and whose owners have no interest in updating to meetstrict.The published packages are 100% TypeScript; the
package.jsonentry point isindex.ts. Only raw.tsfiles are shipped for client developers to use. These sdk files, node_modules/**/*.ts, do need to be complied to.jsfor our final bundle.Issues in the third-party packages seem mostly minor, such as:
21:16 - error TS2532: Object is possibly 'undefined'302:9 error TS2322: Type '(value?: void | PromiseLike<void> | undefined) => void' is not assignable to type '(value?: unknown) => void'.230:11 - error TS2322: Type 'null' is not assignable to type 'Timeout'.
There are seemingly hundreds of errors emitted against third-party libraries when
strictis enabled.
However, having to manually enable strict mode for validation, and search for entries in our source tree is error prone, and difficult to enforce.Reacted by nitram, Dakuan, Adam Spiers, Stein Strindhaug and Reuben SteenekampRyanCavanaugh commented
on Dec 9, 2021 MemberMore actionsicywolfy I don't know what to tell you; this is only one of many ways that a dependency that doesn't want to configure itself properly could cause downstream damage (e.g. they could also be doing something bad at runtime; this is out of everyone's control). If these are public packages I can try talking to them?
bartvanheukelom commented
on Dec 20, 2021 More actionsI've done a bunch of cleanup of my code so that I could enable strict mode, only to find out that 1 library in node_modules ships as .ts instead of .d.ts, and does not compile in strict mode. Had I known in advance that this would be a problem, I might've found another library, but by now, it's already integrated into the project and has "poisoned the well", so to speak. Makes me sad that now I can't enable strict mode for just my own code.
Reacted by Dakuan and Ken Gorabhaving been in a similar situation, i seriously considered adding a postinstall script which patches the libraries inside node_modules...
Reacted by Dakuan and Dawson BotsfordI don't think we should do work to enable a scenario like this that's ultimately doomed to be unacceptably suboptimal.
What are the criteria for "unacceptably" here? It feels to me like this is at high risk of allowing the quest for perfection to become the enemy of the good. It's a lot better to be able to disable
strict*checks in third-party code which is proven to behave well, than to be forced to disable it everywhere.See also #40426 (comment)
Reacted by flexiworld, Abdullah Hilson, Gabriel Nastase, Stein Strindhaug and JonLuca De CaroI came here through a web search, and I think this is what I am running into. It seems I have stricter configuration options enabled than some dependencies of mine (both direct dependencies or transitive ones). TypeScript seems to want to apply my rules to code it finds in
node_modules- code that is not in my control nor developed with my rules in mind. Thus, I am getting compiler errors - ~90 of them, albeit if I fiddle around with combinations ofexcludeandskipLibCheckI can get it down to ~80.I would rather not loosen my configuration to accommodate for code that is not mine, nor do I have any singular influence to tell every project that happens to end up a dependency of mine to develop to my standard. I imagine these projects have their own configurations that their code adheres to, so suspending technical feasibility for a moment would it be a decent idea to only apply the nearest configuration to any given TypeScript file?
I bring it up because the other options seem to be trying to strong-arm library developers into doing X or Y, and regardless of right or wrong in my experience no one takes kindly to trying to be strong-armed. Meanwhile, people who are just trying to work on their own TypeScript projects end up blocked or reducing their own standards due to some dependency that got pulled in somewhere along the way - unless they can fully track down where it came from and search for alternative libraries whose entire dependency tree adheres to the "rules". That latter bit seems like an unfair request to make of people who just want to use TypeScript in their project.
For me, this is a show-stopper as many of these errors come from transitive dependencies whose "ground zero" is a primary technology choice of mine. I am unwilling to just give up my tech choice over this issue, so my options are to shelve the project until resolution or to remove TypeScript from the project. Neither seem all that appealing, but a product built without TypeScript does seem marginally better than no product at all.
Reacted by Abdullah Hilson and JonLuca De Caro
π Search Terms
node_modules
exclude
alwaysStrict
β Viability Checklist
My suggestion meets these guidelines:
β Suggestion
Allow compiler options that enable strict checking to only apply to project files only, and not imported modules.
[Edit 3] It is ideal if this can be expanded to suppress linter checks on node_modules, as well a strict checks.
[Edit 2]
A implementation proposal:
Provide a new configuration option
--noStrictCheckOnNodeModule. This flag will prevent errors emitting on files referenced via**/node_modules/**. This may be further restricted to if the real-path of fire either within**/node_modules/**or outside the Project Root. Project Root may defined as the location of package.json. No additional configuration is possible. All strict mode checks are suppressed for these files.π Motivating Example
A project imports a npm package, that contains .ts files.
Project compiles, and types are coherent.
Attempting to enable
noImplictReturns, finds errors in thenode_modules/in an included project, and fails to compile.The typing in the third-party library is correct:
public static getTrigger(): undefined | "load" => { if (check()) return "load"; }Though, the method has an implicit return.
I see no reason why the project should fail to compile, especially in this case, where the difference between
return undefinedand/* implicit return undefined */is effectively a style issue with no impact on typing or behaviour. Doubly so, in that the errors are found in a third-party library, in code that is otherwise unused.The above is a trivial example, but highlights the issue.
π» Use Cases
The primary use case, is to include strict type checking within a project.
The project in question is a library, and not an application.
The project was originally written with all strict checks enabled.
Over time, as more libraries publish only raw
tsfiles (1), our project had to disable strict checks one by one, in order to compile.At this point in time, the project is unable to enable any strict type checking, or linter checks at all, lest
tscreport errors within thenode_modulesdirectory.The main shortcomings, is that
tsccan no longer be used to enforce strict adherence to guidelines, due to changes in third-party libraries. This has immediate impact on code quality. In particular, being unable to enablestrictNullCheckson our library due to third-party library code, has negatively impacted code quality over time.Currently, no workarounds have been found.
The best effort currently, is to check out a PR; enable all strict typings; sift through pages of reported errors, and attempt to find those not in
node_modules. And then approve or deny based on whether any "real errors" were found. This leaves much room for error, and not all developers or reviewers may be diligent in finding a "real error". Whereas, it would be ideal for these sorts of errors to be reported at develop time, or on the build server.(1) Against guidelines, an increasing number of libraries only publishing raw ts files, without providing any
.jsfiles. In these libraries,package.jsonrefer to.tsfiles directly.{ "module": "./src/main.ts", "types": "./src/main.ts", }(2) This appears to be a common request on stack overflow, and elsewhere:
Change History:
noImplicitReturnwas in a different category thanStrict Checks.