search

microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.94k stars 12.48k forks source link

Stack overflow within collectDynamicImportOrRequireCalls #27433

Open minestarks opened 6 years ago

minestarks commented 6 years ago

TypeScript Version: 3.1.0-dev.20180925

git clone --depth=1 https://github.com/zuiidea/antd-admin.git
cd antd-admin
tsc --init
tsc --allowJs --checkJs

Compiler crashes with the callstack:

RangeError: Maximum call stack size exceeded
    at Object.isRequireCall (node_modules\typescript\lib\tsc.js:7304:27)
    at collectDynamicImportOrRequireCalls (node_modules\typescript\lib\tsc.js:69739:24)
    at visitNode (node_modules\typescript\lib\tsc.js:12689:24)
    at Object.forEachChild (node_modules\typescript\lib\tsc.js:12890:24)
    at collectDynamicImportOrRequireCallsForEachChild (node_modules\typescript\lib\tsc.js:69754:20)
    at collectDynamicImportOrRequireCalls (node_modules\typescript\lib\tsc.js:69748:17)
    at visitNode (node_modules\typescript\lib\tsc.js:12689:24)
    at Object.forEachChild (node_modules\typescript\lib\tsc.js:12890:24)
    at collectDynamicImportOrRequireCallsForEachChild (node_modules\typescript\lib\tsc.js:69754:20)
    at collectDynamicImportOrRequireCalls (node_modules\typescript\lib\tsc.js:69748:17)
sandersn commented 6 years ago

antd-admin includes a machine-generated JS file. The bulk of the file is single gigantic expression that adds a bunch of strings.

There are two ways to fix this:

  1. Put a recursion limiter in collectDynamicImportOrRequireCalls. This is super easy and takes about 4 lines of code. It means that we'll miss import and require calls in expressions that are deeper than (say) 1000 or 2000.
  2. Collect import and require calls in the parser as we encounter them and push them into a list. Store that list on the SourceFile node and just read it directly from there.
sheetalkamat commented 6 years ago

@sandersn option2 might not work with incremental parser?

sandersn commented 6 years ago

@weswigham suggests:

  1. In the parser, parse a flat list for too-large binary expressions, called OperatorListExpression, to be used when there are a lot of binary expressions in a row, all separated by the same operator.
sandersn commented 6 years ago

And, also

  1. Use a language-service-like grep import combined with a filter over all nodes that match.
sandersn commented 6 years ago

I did (4) in the branch fix-overflow-in-collect-dynamic-import, which fixes the original crash.

But a test case now stack overflows in the binder, so I started on (3). This turns out to be a big task, because everywhere we handled BinaryExpression, we now have to handle OperatorListExpression as well. This turns out be quite a few places.