-
Notifications
You must be signed in to change notification settings - Fork 251
Fix SourceMaps not including the original *.ts files #2540
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: MSNev <54870357+MSNev@users.noreply.github.com>
Co-authored-by: MSNev <54870357+MSNev@users.noreply.github.com>
Co-authored-by: MSNev <54870357+MSNev@users.noreply.github.com>
Co-authored-by: MSNev <54870357+MSNev@users.noreply.github.com>
@@ -0,0 +1,186 @@ | |||
// Simple test to verify source map handling in updateDistEsm.js |
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.
Create a test folder called tools/updateDistEsm/src and put all test code in there, also convert to TypeScript so that this can be compiled and added as a task.
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.
Done. Created tools/updateDistEsm/src
folder and moved the test code there as test-sourcemap.ts
with TypeScript conversion and added a tsconfig.json
for compilation. Commit: 1c5681b
export const STR_NOT_DYNAMIC_ERROR = "Not dynamic - "; |
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.
reemove all changes from this PR that are just newline changes at the end of the file.
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.
Removed the newline change from shared/AppInsightsCore/src/JavaScriptSDK/InternalConstants.ts
by reverting to the original state. Commit: 1c5681b
…and remove newline changes Co-authored-by: MSNev <54870357+MSNev@users.noreply.github.com>
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.
Ug!
Issue
Source maps were not including references to the original TypeScript (.ts) files, but rather were referencing the rewritten JavaScript (.js) files from the build process. This issue occurred during the
updateDistEsm
script execution, which modifies JavaScript files to replace tslib references with shims (to support IE).Root Cause
In the
updateDistEsm.js
script, when generating new source maps, it was setting thesource
property to the path of the JavaScript file being processed:This caused the source maps to reference the JavaScript files instead of the original TypeScript files.
Solution
Modified the source map generation process to:
Additional Improvements
Testing
Created a test script (
test-sourcemap.js
) that simulates the source map update process and verifies that TypeScript references are preserved in all scenarios.Fixes #2440.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
googlechromelabs.github.io
node install.mjs
(dns block)https://storage.googleapis.com/chrome-for-testing-public/136.0.7103.92/linux64/chrome-headless-shell-linux64.zip
https://storage.googleapis.com/chrome-for-testing-public/136.0.7103.92/linux64/chrome-linux64.zip
node install.mjs
(http block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.