-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
base: main
Are you sure you want to change the base?
feat(compilerOption): add instantiationDepthLimit and instantiationCountLimit #44997
Conversation
See also #29602 |
There was a problem hiding this 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.
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. |
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. |
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 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. |
I have the same problem while running |
The depth fix mentioned in this article resolved my problem, even when sticking to an initial depth of 1 I would love to know why 😄 |
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 |
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
andinstantiationCountLimit
by their free decisions and responsibilities, instead of hard-coded values (50 and 5000000 for each).Currently, the limitation thresholds are fixed to
50
and5000000
.src/compiler/checker.ts
I think
compilerOptions
would be great solution for these to be configurable.Thanks :)