Skip to content

4.1 RC crashes when new JSX factory is used with incremental mode #41410

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

Closed
billyjanitsch opened this issue Nov 5, 2020 · 7 comments · Fixed by #41467
Closed

4.1 RC crashes when new JSX factory is used with incremental mode #41410

billyjanitsch opened this issue Nov 5, 2020 · 7 comments · Fixed by #41467
Assignees
Labels
Bug A bug in TypeScript Crash For flagging bugs which are compiler or service crashes or unclean exits, rather than bad output Domain: --incremental The issue relates to incremental compilation Domain: JSX/TSX Relates to the JSX parser and emitter Fix Available A PR has been opened for this issue

Comments

@billyjanitsch
Copy link
Contributor

TypeScript Version: 4.1.1-rc (also reproduces in 4.2.0-dev.20201104)

Search Terms: JSX factory incremental

Code

index.tsx:

export default function Foo(): JSX.Element {
  return <div />;
}

tsconfig.json:

{
  "compilerOptions": {
    "incremental": true,
    "jsx": "react-jsx"
  }
}

package.json:

{
  "scripts": {
    "test": "tsc"
  },
  "devDependencies": {
    "typescript": "4.1.1-rc"
  },
  "dependencies": {
    "@types/react": "^16.9.55",
    "react": "^16.14.0"
  }
}

Expected behavior:

Running tsc shouldn't crash.

Actual behavior:

/Users/billy/work/ts-bug/node_modules/typescript/lib/tsc.js:8489
            throw new Error("start < 0");
            ^

Error: start < 0
    at createTextSpan (/Users/billy/work/ts-bug/node_modules/typescript/lib/tsc.js:8489:19)
    at Object.createTextSpanFromBounds (/Users/billy/work/ts-bug/node_modules/typescript/lib/tsc.js:8498:16)
    at getErrorSpanForNode (/Users/billy/work/ts-bug/node_modules/typescript/lib/tsc.js:10803:19)
    at Object.createDiagnosticForNodeFromMessageChain (/Users/billy/work/ts-bug/node_modules/typescript/lib/tsc.js:10711:20)
    at errorOrSuggestion (/Users/billy/work/ts-bug/node_modules/typescript/lib/tsc.js:36053:141)
    at errorOnImplicitAnyModule (/Users/billy/work/ts-bug/node_modules/typescript/lib/tsc.js:37798:13)
    at resolveExternalModule (/Users/billy/work/ts-bug/node_modules/typescript/lib/tsc.js:37757:21)
    at resolveExternalModuleNameWorker (/Users/billy/work/ts-bug/node_modules/typescript/lib/tsc.js:37711:19)
    at resolveExternalModuleName (/Users/billy/work/ts-bug/node_modules/typescript/lib/tsc.js:37706:20)
    at getSymbolAtLocation (/Users/billy/work/ts-bug/node_modules/typescript/lib/tsc.js:64910:32)

Playground Link:

Can't make a playground because 4.1.1-rc isn't available there yet but here's a minimal repo.

Related Issues:

Seems related to #41330 and prior PRs.

@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Crash For flagging bugs which are compiler or service crashes or unclean exits, rather than bad output Domain: --incremental The issue relates to incremental compilation Domain: JSX/TSX Relates to the JSX parser and emitter labels Nov 5, 2020
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 4.1.2 milestone Nov 5, 2020
@weswigham
Copy link
Member

Well that's odd - most error-related incremental bugs also repro with declaration: true (which greatly simplifies testing for them), but not this one. Looks like the incremental behavior is key to this crash. For my own future reference (just in case), a more complete stack trace is:

D:\Github\test\node_modules\typescript\lib\tsc.js:8472
            throw new Error("start < 0");
            ^

Error: start < 0
    at createTextSpan (D:\Github\test\node_modules\typescript\lib\tsc.js:8472:19)
    at Object.createTextSpanFromBounds (D:\Github\test\node_modules\typescript\lib\tsc.js:8481:16)
    at getErrorSpanForNode (D:\Github\test\node_modules\typescript\lib\tsc.js:10786:19)
    at Object.createDiagnosticForNodeFromMessageChain (D:\Github\test\node_modules\typescript\lib\tsc.js:10694:20)
    at errorOrSuggestion (D:\Github\test\node_modules\typescript\lib\tsc.js:36024:141)
    at errorOnImplicitAnyModule (D:\Github\test\node_modules\typescript\lib\tsc.js:37769:13)
    at resolveExternalModule (D:\Github\test\node_modules\typescript\lib\tsc.js:37728:21)
    at resolveExternalModuleNameWorker (D:\Github\test\node_modules\typescript\lib\tsc.js:37682:19)
    at resolveExternalModuleName (D:\Github\test\node_modules\typescript\lib\tsc.js:37677:20)
    at getSymbolAtLocation (D:\Github\test\node_modules\typescript\lib\tsc.js:64856:32)
    at Object.getSymbolAtLocation (D:\Github\test\node_modules\typescript\lib\tsc.js:35477:31)
    at getReferencedFileFromImportLiteral (D:\Github\test\node_modules\typescript\lib\tsc.js:88628:34)
    at getReferencedFiles (D:\Github\test\node_modules\typescript\lib\tsc.js:88640:53)
    at Object.create (D:\Github\test\node_modules\typescript\lib\tsc.js:88715:41)
    at createBuilderProgramState (D:\Github\test\node_modules\typescript\lib\tsc.js:88961:37)
    at Object.createBuilderProgram (D:\Github\test\node_modules\typescript\lib\tsc.js:89481:21)
    at createEmitAndSemanticDiagnosticsBuilderProgram (D:\Github\test\node_modules\typescript\lib\tsc.js:89748:19)
    at Object.createIncrementalProgram (D:\Github\test\node_modules\typescript\lib\tsc.js:91170:16)
    at Object.performIncrementalCompilation (D:\Github\test\node_modules\typescript\lib\tsc.js:91129:33)
    at performIncrementalCompilation (D:\Github\test\node_modules\typescript\lib\tsc.js:93360:29)
    at executeCommandLineWorker (D:\Github\test\node_modules\typescript\lib\tsc.js:93232:17)
    at Object.executeCommandLine (D:\Github\test\node_modules\typescript\lib\tsc.js:93280:20)
    at Object.<anonymous> (D:\Github\test\node_modules\typescript\lib\tsc.js:93553:4)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at Object.<anonymous> (D:\Github\test\node_modules\typescript\bin\tsc:3:1)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:831:12)
    at startup (internal/bootstrap/node.js:283:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:623:3)

@sheetalkamat
Copy link
Member

sheetalkamat commented Nov 6, 2020

This happens when trying to report error as part of getting resolved symbol for the referenced file so we can add reference. I am surprised that even though getSymbolAtLocation asks to ignore error it is not ignored and tries to set it.

@weswigham this happens when we try to get dependencies for the file from the import list which includes pseudo synthesized import literal

@weswigham
Copy link
Member

weswigham commented Nov 6, 2020

Yeah, I think the fix here is probably getting it (resolveExternalModuleName) to not try to report errors on an API request in the first place.

@weswigham
Copy link
Member

weswigham commented Nov 6, 2020

(The fix is simple, the errorOnImplicitAnyModule call should be guarded by a check for moduleNotFoundError, since that indicates if diagnostics are to be reported - I'm just taking awhile to come up with a test case within our harness - incremental mode tests are always rough for me)

@sheetalkamat
Copy link
Member

@weswigham you can write unittest case in https://github.com/microsoft/TypeScript/blob/master/src/testRunner/unittests/tsc/incremental.ts#L3 use verifyTsc method instead of serialized edits since you are not looking for edits to verify this

@elodszopos
Copy link

I ran into this too - can confirm the initial report. Removing incremental mode lets me run a typecheck without emitting, still breaks down for a build.

Using [email protected].

@Jack-Works
Copy link
Contributor

--watch without incremental in tsconfig also crashes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Crash For flagging bugs which are compiler or service crashes or unclean exits, rather than bad output Domain: --incremental The issue relates to incremental compilation Domain: JSX/TSX Relates to the JSX parser and emitter Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants