[mstange]

Main | Deploy preview

profiler-edit wasn't applying the source map from the VideoSite-Polymer workload.

This workload's source map is an "index source map". This happened more or less by accident; the workload has a build script which does the following:

  // ...
  const chunks = ["base", "main"].map(c => fs.readFileSync(r(`out/chunks/${c}.js`), "utf8"));
  fs.mkdirSync(r("dist"), { recursive: true });
  fs.writeFileSync(r("dist/app.js"), OUTER_PREFIX + chunks.join("\n") + OUTER_SUFFIX + "//# sourceMappingURL=app.js.map\n");
  // An index source map for dist/app.js
  let line = OUTER_PREFIX.split("\n").length - 1;
  const sections = chunks.map((text, i) => {
    const s = { offset: { line, column: 0 }, map: JSON.parse(fs.readFileSync(r(`out/chunks/${["base", "main"][i]}.js.map`), "utf8")) };
    line += text.split("\n").length;
    return s;
  });
  const indexMap = JSON.stringify({ version: 3, file: "app.js", sections });

So the index source map in this case isn't really the output from a single bundler, it's the result of some custom stitching.

Either way, there doesn't seem to be a reason to reject such source maps. We already resolve them correctly when you gather a profile of that workload in the browser, it was just the "apply source map" UI button and profiler-edit which were rejecting them.

And Claude says that Closure can output index source maps in some cases.

[codecov]

Codecov Report

❌ Patch coverage is 89.47368% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.49%. Comparing base (439a940) to head (20e6f3c).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/app-logic/browser-connection.ts	0.00%	1 Missing ⚠️
src/profile-logic/source-maps/matching.ts	96.87%	1 Missing ⚠️
src/profile-query/function-annotate.ts	0.00%	1 Missing ⚠️
src/utils/query-api.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6399   +/-   ##
=======================================
  Coverage   84.48%   84.49%           
=======================================
  Files         357      357           
  Lines       38675    38693   +18     
  Branches    10957    10867   -90     
=======================================
+ Hits        32675    32692   +17     
- Misses       5565     5566    +1     
  Partials      435      435           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[canova]

Ah yeah. I was considering supporting this while working on the source map support, but I decided not to support it because it's not used almost anywhere, and didn't want to complicate it for that.

But looking at the PR, it's not bad actually, so it's worth accepting.

And Claude says that Closure can output index source maps in some cases.

For the record, I remember doing some research about it then and I was confused at first, because at first I thought this was Clojure (the language) that supports it, but this is the Google Closure Compiler. Apparently it's a compiler that compiles (transpiles?) JS into a better JS? And when I checked, that compiler was defaulting to regular/flat source map v3, and there was no cli flag to change it to index maps. And index map can only be output via their Java API, so it still looks pretty unlikely to see it in the wild. But I guess we never know if someone else will want to support it in the future in one of their tools. And considering that this is not much to maintain I think we can add it.

This workload's source map is an "index source map". This happened more or less by accident; the workload has a build script which does the following:

Oh wow, interesting! 😄 Not for this PR ofc but I would also very much prefer if we can change that worload to output regular flat source maps and not index maps 😄

[canova]

Hm currently we do a recursion here for index maps to see if their sections are also source maps, but if I'm not mistaken, an index map can't have an index map as its inner section. But this still accepts that case.

[mstange]

Oh good point. I've changed it and added a test. The library allows recursively nested index source maps but the spec does not.

[mstange]

this is the Google Closure Compiler. Apparently it's a compiler that compiles (transpiles?) JS into a better JS?

Yes, it is used a lot for Google internal sites (YouTube being one of them), and the React minified production bundle is also built with Closure (though they don't use the advanced annotations that could get them more aggressive minification).

that compiler was defaulting to regular/flat source map v3, and there was no cli flag to change it to index maps. And index map can only be output via their Java API, so it still looks pretty unlikely to see it in the wild.

Ah, indeed. Yes I agree it seems unlikely to appear in the wild.

This workload's source map is an "index source map". This happened more or less by accident; the workload has a build script which does the following:

Oh wow, interesting! 😄 Not for this PR ofc but I would also very much prefer if we can change that worload to output regular flat source maps and not index maps 😄

Hmm I can make that change but I am not entirely convinced.

[canova]

Hmm I can make that change but I am not entirely convinced.

Why do you think an index map is better for this case? Is there a specific reason?