Better handling for manually created source files and compiler APIs

[JoshuaKGoldberg]

Search Terms

type checker source file cannot ready property 'members' of undefined

Suggestion

When a user of the compiler API manually creates a source file with ts.createSourceFile instead of retrieving it from their program, and then asks the program for type inference on a contained node, this can crash. See #8136 / vuejs/vue-cli#2712 / angular/tsickle#151 / budgielang/ts-budgie#39.

It seems like one of three interpretations might be best:

• This is explicitly unsupported behavior, but for the sake of performance & simplicity, no checks should happen
• This is explicitly unsupported behavior, and a more explicit error should be thrown
• This should become supported behavior, and the program should dynamically create source files as requested

Use Cases

Auto-generated TypeScript files, such as .vue snippets, still want access to a type checker, such as for TSLint rules.

Examples

palantir/tslint#4273

Checklist

My suggestion meets these guidelines:

• This wouldn't be a breaking change in existing TypeScript / JavaScript code
• This wouldn't change the runtime behavior of existing JavaScript code
• This could be implemented without emitting different JS based on the types of the expressions
• This isn't a runtime feature (e.g. new expression-level syntax)

[weswigham]

This is by design currently, but we should probably attempt to do something better than crash with a random js error (ideally we'd capture the difference between the trees in the API somehow) - maybe we can set a bit on synthetic source files that only gets unset when it becomes correctly bound and checked, and we use that bit to throw early on in the checker if it's set? I'm unsure.

[ajafff]

maybe we can set a bit on synthetic source files that only gets unset when it becomes correctly bound and checked, and we use that bit to throw early on in the checker if it's set?

That probably won't work for SourceFiles from different Programs or Nodes that were previously part of a SourceFile but no longer are because of incremental parsing through updateSourceFile.

[weswigham]

We could set a random ID (random as in non-sequential, but the same ID for every node) on all nodes in a program and assert if the node the checker receives has an ID not equal to the hosting program's, perhaps?

[JoshuaKGoldberg]

@weswigham I haven't come up with a better idea than that 😄. Would you accept a PR for the program-specific IDs?

Clarifying questions:

• Would each round of parsing (first the initial creation, then incremental parsing) use a new ID created by, say Math.random() + "-" + ts.timestamp()?
• Is the performance cost of updating all nodes something you're concerned about? Meaning: would you want a PR to include timing tests showing no real change, and if so, how do you TypeScript folks normally create those?

Edit: oh, and should this be an opt-in feature, so that the CLI doesn't enable it, but external tools programmatically calling TS APIs would?

[weswigham]

Would each round of parsing (first the initial creation, then incremental parsing) use a new ID

I'd say so, yes.

Is the performance cost of updating all nodes something you're concerned about?

Setting it at the same time we refresh and set .parent pointers in the parser/binder should be relatively cheap, since those need to be rewritten in the same way.

Edit: oh, and should this be an opt-in feature, so that the CLI doesn't enable it, but external tools programmatically calling TS APIs would?

I'd be temped to tie it to the diagnostics flag - but it's probably best to only tie it to a flag if it has measurable perf impact.

Meaning: would you want a PR to include timing tests showing no real change, and if so, how do you TypeScript folks normally create those?

We have some internal tools for informally measuring perf changes of some samples we have (and analyzing the resulting perf traces) - unfortunately we've never made them public. The closest thing we use in public is the aggregate timing data collected by the --extendedDiagnostics flag, but that's only for tsc. For measuring, eg, the impact on LS operations and incremental parse, we don't have anything anywhere (at least that I know of). We store execution time of tests when they're run in parallel so we can more optimally schedule them on future runs, so that tracks, eg, fourslash test duration. You could maybe shoehorn that into measuring a fourslash test that edits and rechecks a file a bunch of times (or just enable and use the ts.performance timers on fourslash tests and add a new verifier/baseliner)... but yeah - we have no formal infra for measuring LS perf generally right now. When a perf issues is reported, we usually just get a repro and then open the build or LS in the chrome devtools and run a performance trace - that's not great for trying to benchmark a LS change, though.

[backbone87]

While i am not too familar with typescript internals, the fork-ts-checker-webpack-plugin, somehow managed to allow typechecking for .vue file: https://github.com/Realytics/fork-ts-checker-webpack-plugin/blob/master/src/VueProgram.ts