GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add maxInstantiationDepth to compiler optionDeclarations by AnyhowStep · Pull Request #29602 · microsoft/TypeScript

Conversation

@AnyhowStep

@AnyhowStep AnyhowStep commented Jan 26, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
  • There is an associated issue that is labeled
    'Bug' or 'help wanted' or is in the Community milestone
  • Code is up-to-date with the master branch
  • You've successfully run jake runtests locally (Had to run jake baseline-accept)
  • You've signed the CLA
  • There are new or updated unit tests validating the change

Fixes #29511

image

The first two make sense to me, since I added a new field to the CompilerOptions interface in types.ts

The last one isn't so obvious to me and I'm not sure how I'd go about correcting this.

[EDIT]
Oh, I think it's just that I have to accept the new baseline file under baselines/local/showConfig/Shows tsconfig for single option/maxInstantiationDepth/tsconfig.json

{
    "compilerOptions": {
        "maxInstantiationDepth": 0
    }
}

I looked at the baseline changes and they seem okay to me. Nothing wildly different. So, I went and accepted them.

I know it's just a test with a dummy value but setting maxInstantiationDepth to 0 is a bad idea =P
[/EDIT]


The issue is only marked as Needs Investigation but I figured I'd take a stab at adding this compiler option. Being able to increase the maximum instantiation depth from 50 to an arbitrary value is useful for me because I seem to work with deeply nested, non-recursive, types a lot.

I'm not sure how to write unit tests to test this compiler option yet but I'll try and figure it out. I figured I'd just make the PR first.

I'm not sure what description to give this compiler option, and what code it would use.

Do I need to add maxInstantiationDepth to protocol.ts > CompilerOptions, too?

I'm also unsure what else I might be missing.


While jake runtests fails and I haven't written a unit test for this compiler option, I've tested it against a project I'm working on and it seems to work.

Without setting maxInstantiationDepth, compiling my project fails, setting maxInstantiationDepth to 74 lets it compile successfully.


I realize this PR has a bunch of problems but I'm out of my depth (No pun intended).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

matthargett, timroes, GKerkhoveKsyos, castarco, and ForkKILLET reacted with heart emoji
All reactions
  • ❤️ 5 reactions
AnyhowStep added 2 commits January 26, 2019 00:58
@AnyhowStep
Add maxInstantiationDepth to optionDeclarations
a6f27cb
@AnyhowStep
Ran baseline-accept, changes seem reasonable
f2e7c94
@AnyhowStep AnyhowStep mentioned this pull request Jan 29, 2019
Long conditional type resolves to any #28663
Closed
@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) added the Experiment A fork with an experimental idea which might not make it into master label Apr 25, 2019
@weswigham Wesley Wigham (weswigham) mentioned this pull request Jul 29, 2019
Use generator trampolines during instantiation #32611
Closed
@AnyhowStep AnyhowStep mentioned this pull request Nov 11, 2019
“Type instantiation is excessively deep and possibly infinite” but only in a large codebase #34933
Open
@sandersn

Nathan Shively-Sanders (sandersn) commented Feb 3, 2020

Copy link
Copy Markdown
Member

maxInstantiationDepth is an intentional limitation of the compiler to prevent slow compiles, so we're not going to remove it. Even if #32611 allows much more nested types to compile, it's likely that we'll have a different kind of limiter in place. Because that future limiter will be named something different, I don't think PR will apply to it.

groege, orzFly, SancheZz, prztrz, jespertheend, dermotduffy, ashlog, Gotos, giladgd, brandonwkipp, and 9 more reacted with thumbs down emoji
All reactions
  • 👎 19 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sandersn Nathan Shively-Sanders (sandersn) closed this Feb 3, 2020
@kiprasmel Kipras Melnikovas (kiprasmel) mentioned this pull request Jul 30, 2021
feat(compilerOption): add instantiationDepthLimit and instantiationCountLimit #44997
Closed
@benbuzbee

Ben Buzbee (benbuzbee) commented Oct 21, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

maxInstantiationDepth is an intentional limitation of the compiler to prevent slow compiles, so we're not going to remove it. Even if #32611 allows much more nested types to compile, it's likely that we'll have a different kind of limiter in place. Because that future limiter will be named something different, I don't think PR will apply to it.

I am surprised by the feedback; TypeScript's flexibility has always been its strength.

If a codebase and an engineering team feel it could benefit more from a complex typing structure that enables something that may not be otherwise enabled with full compile time type safety; why shouldn't they be empowered to make the tradeoff choice themselves about whether they will accept slow compiles?

Especially when the change seems so easy and clean.

Perhaps there is more to the story than I see though; would you be open to further discussion?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

Experiment A fork with an experimental idea which might not make it into master

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[API Question] Is there a type resolution depth limit difference between compiling and running the language service?

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@AnyhowStep @sandersn @benbuzbee @RyanCavanaugh
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.