strip out git askpass sourcemap footer#291673
Conversation
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @lszomoruMatched files:
|
There was a problem hiding this comment.
Pull request overview
Adds a build-time mechanism to remove the //# sourceMappingURL=... footer from specific bundled extension artifacts (currently Git’s askpass-main.js) to address the terminal environment-variable churn reported in #282020.
Changes:
- Introduces a
StripOutSourceMapsexport in the Git extension webpack config to markdist/askpass-main.jsfor special handling. - Updates the extension packaging pipeline (
build/lib/extensions.ts) to either rewritesourceMappingURLto the CDN (existing behavior) or remove it entirely for configured files.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| extensions/git/extension.webpack.config.js | Declares dist/askpass-main.js as a bundle whose sourcemap footer should be stripped. |
| build/lib/extensions.ts | Reads StripOutSourceMaps from extension webpack configs and conditionally removes vs rewrites the sourceMappingURL footer during packaging. |
| data.contents = Buffer.from(contents.replace(/\n\/\/# sourceMappingURL=(.*)$/gm, function (_m, g1) { | ||
| return `\n//# sourceMappingURL=${sourceMappingURLBase}/extensions/${path.basename(extensionPath)}/${relativeOutputPath}/${g1}`; | ||
| }), 'utf8'); | ||
| if (stripOutSourceMaps.indexOf(data.relative) >= 0) { // remove source map |
There was a problem hiding this comment.
data.relative is derived from path.relative(...) (via Vinyl) and will use platform-specific path separators (e.g. dist\\askpass-main.js on Windows). Since StripOutSourceMaps values are authored with POSIX separators (dist/askpass-main.js), this comparison won’t match on Windows builds and the sourceMappingURL footer won’t be stripped. Normalize both sides (e.g. path.normalize(...) when collecting and comparing, or convert to POSIX with replace(/\\/g, '/')) and consider using a Set + has() for clarity.
| if (stripOutSourceMaps.indexOf(data.relative) >= 0) { // remove source map | |
| const stripOutSourceMapsSet = new Set(stripOutSourceMaps.map(p => path.normalize(p))); | |
| const normalizedRelativePath = path.normalize(data.relative); | |
| if (stripOutSourceMapsSet.has(normalizedRelativePath)) { // remove source map |
fixes #282020