Repository navigation
sourcemap mapping is generated before node generates newline and indent #39170
Description
TypeScript Version: 4.0.0-dev.20200615
Search Terms:
sourcemap
Code
export type AsyncFn<TResult> = () => PromiseLike<TResult>;
export type RaceCancelFn = <TResult>(
asyncFnOrPromise: AsyncFn<TResult> | PromiseLike<TResult>
) => Promise<TResult>;
export type DisposeFn = () => void;
export type DisposableExecutorFn<TResult> = (
resolve: (value?: TResult | PromiseLike<TResult>) => void,
reject: (reason?: any) => void
) => DisposeFn;
const noopRaceCancel: RaceCancelFn = <TResult>(
asyncFnOrPromise: AsyncFn<TResult> | PromiseLike<TResult>
): Promise<TResult> =>
typeof asyncFnOrPromise === "function"
? Promise.resolve().then(asyncFnOrPromise)
: Promise.resolve(asyncFnOrPromise);
export default async function cancellablePromise<TResult>(
disposableExecutor: DisposableExecutorFn<TResult>,
raceCancel: RaceCancelFn = noopRaceCancel
): Promise<TResult> {
let dispose: DisposeFn | undefined;
try {
return await raceCancel(
() =>
new Promise<TResult>((resolve, reject) => {
dispose = disposableExecutor(resolve, reject);
})
);
} finally {
if (dispose !== undefined) {
dispose();
}
}
}copy above code to index.ts then run:
tsc --target esnext --sourceMap index.ts
Expected behavior:
I expect that since this should just be removing types that the generated sourcemap could map all JS AST nodes start/end back to the original but at a minimum starts of statements, expressions, identifiers, blocks. Ideally for source map generation identifiers would 1 to 1 map to identifiers and not include type. Ideally starts should not include whitespace.
Actual behavior:
Missing closing braces in the try/finally block and the if in the finally block (since tsc 2.8).
Closing parens often start on prior lines ending newline instead of the paren in the original (since as far back as tsc 2.0).
Identifiers missing mapping at end when typing info was removed or there are default expressions.
The last one maybe considered a nice to have but is helpful with various tooling and to survive downstream transforms like rollup and minification.
Activity
krisselden commentedon Jun 19, 2020
Example of the issue the missing braces cause in code coverage
https://coveralls.io/builds/31527629/source?filename=race-cancellation/src/cancellablePromise.ts#L23
krisselden commentedon Jun 20, 2020
My wild guess here is that mappings aren't emitted for the end of the generated AST nodes and often this works out because the next thing's start often works as an end. Not having the end be mappable is problematic as a lot of tooling tries to map the end as well and it is often missing.
krisselden commentedon Jun 20, 2020
More context for why AST node end is important to map:
https://github.com/rollup/rollup/blob/2508884ab604df269c5450f7994faaf5e98873a9/src/ast/nodes/shared/Node.ts#L114
https://github.com/istanbuljs/v8-to-istanbul/blob/6592f82333944e1696ff61079728cde8e650b256/lib/v8-to-istanbul.js#L159
krisselden commentedon Jun 22, 2020
After adding some instrumentation, it is clear what the issues are that are causing the above.
First, the ends of identifiers are skipped. Second, there are newlines and whitespace inserted after the start mapping is emitted and/or before the end mapping is emitted.
krisselden commentedon Jun 23, 2020
Ryan Cavanaugh (@RyanCavanaugh) I did a spike of a relatively simple fix to tsc.js that addressed most the issues above (all the invalid mappings). If I turned this into a PR, any chance of getting into the 3.x branch? I see this got labelled as 4.
Most the invalid mappings are caused by emitting a mapping for then next pos to be written then writeLine() being called which can be fixed by queuing adding the mapping until the first write method that isn't writeLine() and the remaining invalid mappings were caused by the src pos pointing to a newline which can be fixed by checking if it is a newline then forwarding it past the newline to the first non whitespace character. This fixes majority of all issues I know of.
RyanCavanaugh commentedon Jun 23, 2020
Kris Selden (@krisselden) unless this is a regression, we'd generally only backport for critical bugs, which I don't think this qualifies
krisselden commentedon Jul 15, 2020
Workaround to post process the out of bound column mappings to the next line + indent https://github.com/krisselden/race-cancellation/blob/master/race-cancellation/scripts/fix-maps.cjs
[-]try finally block broken sourcemap[/-][+]sourcemap mapping is generated before node generates newline and indent[/+]8 remaining items
krisselden commentedon Sep 13, 2022
Ron Buckton (@rbuckton) the parenthesis is only part of the issue, the other part of this issue is newline and intent is applied after the mapping is emitting which breaks the mapping as it puts it on a new line and the col of the emitted mapping is out of bounds.
I got that part to work by post processing the out of bound mapping wrapping onto the next line. Then code coverage could map back to the TS.
I would be nice to add a test of using common build tooling that chains through source maps, like terser and c8.