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

feat(compilerOption): add instantiationDepthLimit and instantiationCountLimit #44997

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jjangga0214
Copy link
Contributor

@jjangga0214 jjangga0214 commented Jul 13, 2021

Fixes #34933.

I am not familiar with this project structure. This PR might be incomplete, so I hope someone contributes to complete it.
I only edited 3 files, and didn't touch Diagnostics though I referred to new fields from it.

Please let users be able to adjust instantiationDepthLimit and instantiationCountLimit by their free decisions and responsibilities, instead of hard-coded values (50 and 5000000 for each).

Currently, the limitation thresholds are fixed to 50 and 5000000.

src/compiler/checker.ts

if (
  instantiationDepth === 50 ||
  instantiationCount >= 5000000
) {
  // We have reached 50 recursive type instantiations and there is a very high likelyhood we're dealing
  // with a combination of infinite generic types that perpetually generate new type identities. We stop
  // the recursion here by yielding the error type.
  error(
    currentNode,
    Diagnostics.Type_instantiation_is_excessively_deep_and_possibly_infinite
  );
  return errorType;
}

I think compilerOptions would be great solution for these to be configurable.

{
  "compilerOptions": {
    "instantiationDepthLimit": 100,
    "instantiationCountLimit": 10000000
  }
}

Thanks :)

@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Jul 13, 2021
@ghost
Copy link

ghost commented Jul 13, 2021

CLA assistant check
All CLA requirements met.

@kiprasmel
Copy link

See also #29602

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahejlsberg plans to increase the limit in #45025, but so far we don't want to make any limits configurable, since they're such fluctuating implementation details.

@jjangga0214
Copy link
Contributor Author

jjangga0214 commented Aug 14, 2021

@sandersn

Though it can be viewed as an implementation detail, there're high demands for configuration (See thumbs and overall attention of people).

What's more, There're still cases the increased limit can't cover.

Thus, how about experimental configuration?

P.S. Actually I've already left a comment a month ago about experimental configuration on #45025, but nobody replied to this idea.

@RyanCavanaugh
Copy link
Member

I can't stress enough that we don't think this is appropriate to expose as configuration. Today instantiation is a process with a particular memory and runtime cost that occurs during certain operations, but every aspect of that is an implementation detail that could change tomorrow, at which point the numeric limits specified at any given point in time do not translate to the same behavior any more.

This isn't a fix for the linked issue and, again, we do not consider this to be a "tunable" parameter.

@RyanCavanaugh RyanCavanaugh marked this pull request as draft August 17, 2021 20:08
@Jamesgt
Copy link

Jamesgt commented Jan 20, 2022

Ok, so you don't consider it, but then I guess more and more "solution" will appear.

Since I am not allowed to use a patched tsc, to compile my code I had to use // @ts-ignore (see https://stackoverflow.com/a/64688342/146334) to have my complex mongodb schema built.

I don't see why it is better to mute errors (with known side effects) instead of allowing parameters tuning for users. The default value can stay in your ownership, so 99.999% of users will use that. If even in a patch version, the behavior changes, we still can adapt to it using the same, or new experimental configuration.

@danielo515
Copy link

I have the same problem while running typeof someJsonObject where the json object has something around 3k properties.
Does anyone know at least where is the limit? Or should I go and test recursively until I find it?

@Ricardo-Marques
Copy link

The depth fix mentioned in this article resolved my problem, even when sticking to an initial depth of 1
https://www.angularfix.com/2022/01/why-am-i-getting-instantiation-is.html

I would love to know why 😄

@nopeless
Copy link

I can't stress enough that we don't think this is appropriate to expose as configuration. Today instantiation is a process with a particular memory and runtime cost that occurs during certain operations, but every aspect of that is an implementation detail that could change tomorrow, at which point the numeric limits specified at any given point in time do not translate to the same behavior any more.

This isn't a fix for the linked issue and, again, we do not consider this to be a "tunable" parameter.

Sorry for commenting on an old post, but changing the parameters to something larger temporarily would allow the developer to determine whether their type is actually recursive or infinite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

“Type instantiation is excessively deep and possibly infinite” but only in a large codebase
10 participants