Skip to content

Stop generating sectional sourcemaps#24917

Merged
weswigham merged 12 commits into
microsoft:masterfrom
weswigham:stop-generating-sectional-sourcemaps
Jun 14, 2018
Merged

Stop generating sectional sourcemaps#24917
weswigham merged 12 commits into
microsoft:masterfrom
weswigham:stop-generating-sectional-sourcemaps

Conversation

@weswigham
Copy link
Copy Markdown
Member

@rbuckton started working with the index maps we generated in our tooling and found that while many tools profess to handle index maps, they don't handle them particularly well (ie, the mozilla source map library crashes). This change causes us to process the incoming sourcemaps and add the offsets and updated paths required to emit a traditional single section sourcemap, thereby improving compatibility with those tools (at the cost of more work performed during sourcemap emit).

@weswigham
Copy link
Copy Markdown
Member Author

Incidentally, this means we also wouldn't need to update our sourcemap decoding service to handle index maps just to support project references sourcemaps. 🤷‍♀️

Comment thread src/compiler/sourcemap.ts Outdated
resetSectionalData();
}
}
function isReasonablySourceMap(x: {}): x is SourceMapSection {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is Probably SourceMap ?

Copy link
Copy Markdown
Contributor

@rbuckton rbuckton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general this is ok. However, I am concerned about the resulting paths in the merged sourcemap. #24920 should help in diagnosing that.

Comment thread src/compiler/sourcemap.ts
encodeLastRecordedSourceMapSpan();

return JSON.stringify(generateMap());
return JSON.stringify(captureSection());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we aren't doing indexed maps any more, we should probably remove references to names like "Section", etc.

Copy link
Copy Markdown
Contributor

@rbuckton rbuckton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The source map paths look correct now.

Comment thread src/compiler/factory.ts
}

export function createUnparsedSourceFile(text: string, map?: string): UnparsedSource {
export function createUnparsedSourceFile(text: string, mapPath?: string, map?: string): UnparsedSource {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid possible issues with existing calls, and since these aren't actually optional, we should have the types be string | undefined just to make sure we didn't miss any calls.

Comment thread src/compiler/factory.ts
}

export function createInputFiles(javascript: string, declaration: string, javascriptMapText?: string, declarationMapText?: string): InputFiles {
export function createInputFiles(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid possible issues with existing calls, and since these aren't actually optional, we should have the types be string | undefined just to make sure we didn't miss any calls.

@weswigham weswigham merged commit 2a15036 into microsoft:master Jun 14, 2018
@weswigham weswigham deleted the stop-generating-sectional-sourcemaps branch June 14, 2018 01:52
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants