-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: ensure sourcemap references use correct .js.map extension #8345
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
Conversation
- Modified sourcemapPlugin to detect and fix incorrect sourcemap references - Ensures all JavaScript files reference .js.map files instead of .sourcemap - Fixes issue where checkpoint restore buttons were not displaying due to sourcemap loading errors Fixes #8344
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self-review: auditing my own code is like linting a mirror—reflective, pedantic, and slightly unsettling.
Summary:
- P1: Unintended gitlinks added (.review/pr-8274, tmp/pr-8287-Roo-Code). Please remove from the PR; consider .gitignore if needed.
- P2: Don’t overwrite inline data URL sourcemaps; guard with currentMapRef.startsWith("data:").
- P3: Prefer atomic writes for JSON maps (tmp + rename) to avoid partial writes.
| // Check if the source map is already referenced | ||
| if (!jsContent.includes("//# sourceMappingURL=")) { | ||
| // Check if the source map is already referenced or has incorrect reference | ||
| const sourceMappingURLRegex = /\/\/# sourceMappingURL=([^\s\n]+)/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: Regex robustness: anchor to the line and use multiline to avoid partial matches.
| const sourceMappingURLRegex = /\/\/# sourceMappingURL=([^\s\n]+)/ | |
| const sourceMappingURLRegex = /^\/\/# sourceMappingURL=(.+)$/m |
| const currentMapRef = match[1] | ||
| const expectedMapRef = `${jsFile}.map` | ||
|
|
||
| if (currentMapRef !== expectedMapRef) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Avoid overwriting inline data URL sourcemaps (data:...). Guard before replacing.
| if (currentMapRef !== expectedMapRef) { | |
| if (!currentMapRef.startsWith("data:") && currentMapRef !== expectedMapRef) { |
|
Closing as stale |
Description
This PR fixes an issue where sourcemap references were using incorrect file extensions (e.g.,
index.sourcemapinstead ofindex.js.map), which prevented checkpoint restore buttons from displaying when hovering over checkpoint lines.Problem
index.sourcemapinstead ofindex.js.mapSolution
Modified the
sourcemapPluginto:.sourcemap→.js.map)Changes
webview-ui/src/vite-plugins/sourcemapPlugin.tsto detect and fix incorrect sourcemap references.js.mapfiles correctlyTesting
Related Issue
Fixes #8344
Review Confidence
The implementation review showed 92% confidence with no security issues and good code quality adherence.
Future Improvements
Consider adding unit tests for the sourcemap plugin to ensure the fix works correctly across all scenarios.
Important
Fixes sourcemap references in
sourcemapPlugin.tsto ensure.js.mapextension is used, resolving checkpoint restore issues.sourcemapPlugin.tsto use.js.mapextension instead of.sourcemap..js.mapextension.This description was created by
for f872000. You can customize this summary. It will automatically update as commits are pushed.