Skip to content

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented May 20, 2025

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 the source property to the path of the JavaScript file being processed:

var newMap = theString.generateMap({
    source: inputFile.toString(),  // This was incorrectly pointing to the JS file
    file: mapFile,
    includeContent: true,
    hires: false
});

This caused the source maps to reference the JavaScript files instead of the original TypeScript files.

Solution

Modified the source map generation process to:

  1. Read the existing source map to extract the original TypeScript sources
  2. Generate a new source map with the modifications
  3. Preserve the original sources and sourcesContent from the existing map
  4. Update the map file with these preserved sources

Additional Improvements

  • Added robust error handling for cases where source maps don't exist or are invalid
  • Ensured the code works correctly in all scenarios (existing maps, missing maps, corrupted maps)
  • Added comprehensive test cases to verify all edge cases:
    • Preserving TypeScript source references in existing maps
    • Handling cases where source maps don't exist
    • Gracefully recovering from corrupted/invalid source maps

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
    • Triggering command: 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
    • Triggering command: 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.

Copilot AI and others added 4 commits May 20, 2025 22:56
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>
@Copilot Copilot AI changed the title [WIP] [BUG] SourceMaps are not including the original *.ts files Fix SourceMaps not including the original *.ts files May 20, 2025
@Copilot Copilot AI requested a review from MSNev May 20, 2025 23:04
Copilot finished work on behalf of MSNev May 20, 2025 23:04
sourcesContent: [jsContent2]
};

fs.writeFileSync(mapFile2, JSON.stringify(generatedMap2));

Check failure

Code scanning / CodeQL

Potential file system race condition High test

The file may have changed since it
was checked
.
@@ -0,0 +1,186 @@
// Simple test to verify source map handling in updateDistEsm.js
Copy link
Collaborator

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.

Copy link
Contributor Author

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 - ";
Copy link
Collaborator

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.

Copy link
Contributor Author

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>
Copilot finished work on behalf of MSNev May 28, 2025 20:29
Copy link
Collaborator

@MSNev MSNev left a comment

Choose a reason for hiding this comment

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

Ug!

@MSNev MSNev added the keep Do not Mark as Stale and close label May 28, 2025
@MSNev MSNev added delayed investigation required Further investigation or discussions required labels May 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delayed investigation required Further investigation or discussions required keep Do not Mark as Stale and close
Projects
None yet
Development

Error loading sessions

Retrying.

Successfully merging this pull request may close these issues.

[BUG] SourceMaps are not including the original *.ts files
2 participants