Skip to content

Add startLine, startColumn, sourceMapURL and rename uuid to id in source table #5882

Merged
canova merged 5 commits intofirefox-devtools:mainfrom
canova:source-table-improvements
Mar 9, 2026
Merged

Add startLine, startColumn, sourceMapURL and rename uuid to id in source table #5882
canova merged 5 commits intofirefox-devtools:mainfrom
canova:source-table-improvements

Conversation

@canova
Copy link
Member

@canova canova commented Mar 4, 2026

The backend side of this PR:

This PR improves the source table overall by:

  • Adding startLine and startColumn to the source table. This is going to be used for showing correct line numbers in the source view when the source is coming from an inline script. And also it will be useful for source map/prettify in the future.
  • Adding sourceMapURL if the source contains this information. This is needed for us implement the source map resolution.
  • Renaming uuid field to id. This field was initially containing uuid fields, but they were replaced to contain hashes instead. It's better to have an implementation agnostic name for this field in general.

It's a bit difficult to test as it request profiling with the backend patches though. That's why it might be difficult to get a profile for deploy preview.

Also there isn't a UI visible change in this PR yet. The following PRs will use startLine/startColumn, and then sourceMapURL will be used for source map fetching.

@canova canova force-pushed the source-table-improvements branch from 81eb713 to bba65ec Compare March 4, 2026 17:47
@canova canova requested review from fatadel and mstange March 4, 2026 17:48
@codecov
Copy link

codecov bot commented Mar 4, 2026

Codecov Report

❌ Patch coverage is 81.31868% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.41%. Comparing base (23fea36) to head (8f73bab).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
src/profile-logic/process-profile.ts 0.00% 9 Missing ⚠️
src/profile-logic/gecko-profile-versioning.ts 81.25% 3 Missing ⚠️
src/profile-logic/profile-compacting.ts 72.72% 2 Missing and 1 partial ⚠️
src/profile-logic/global-data-collector.ts 93.75% 1 Missing ⚠️
src/profile-logic/merge-compare.ts 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5882      +/-   ##
==========================================
- Coverage   85.42%   85.41%   -0.02%     
==========================================
  Files         321      321              
  Lines       32073    32140      +67     
  Branches     8821     8757      -64     
==========================================
+ Hits        27398    27451      +53     
- Misses       4244     4257      +13     
- Partials      431      432       +1     

☔ View full report in Codecov by Sentry.
📢 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.

@fatadel
Copy link
Contributor

fatadel commented Mar 5, 2026

Nit: when writing in camelCase, I would prefer having sourceMapUrl instead of sourceMapURL (lower case r and l). See: https://google.github.io/styleguide/jsguide.html#naming-camel-case-defined.

@canova
Copy link
Member Author

canova commented Mar 5, 2026

Nit: when writing in camelCase, I would prefer having sourceMapUrl instead of sourceMapURL (lower case r and l). See: https://google.github.io/styleguide/jsguide.html#naming-camel-case-defined.

That's valid, but I wanted to keep sourceMapURL consistent with its SpiderMonkey implementation: https://searchfox.org/firefox-main/rev/22d04b52b0eb8d9fa11bf8ede5ccc0243a07c5ba/js/src/vm/JSScript.cpp#2224
I would prefer to keep it consistent for being able to grep accurately across different codebases.

@canova canova force-pushed the source-table-improvements branch from bba65ec to bdcbeb6 Compare March 5, 2026 12:26
Copy link
Contributor

@fatadel fatadel left a comment

Choose a reason for hiding this comment

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

I've left some "nit" comments, but otherwise looks great. Thanks for the PR!

Copy link
Contributor

@mstange mstange left a comment

Choose a reason for hiding this comment

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

r+ with the bug fixed

canova added 3 commits March 9, 2026 14:04
This will help us see the accurate line numbers in the source view for
the inline JS scripts. And also, it will help us resolve the source maps
later.
This will allow us to resolve the source maps later.
The name of this field doesn't make sense anymore as we changed its
value from UUID to a hash value. This `id` name is more implementation
agnostic, so it's better to rename it now.

Note that this requires an upgrader, similar to the previous patches in
this patch stack. The upgrader will happen in the last commit with all
the relevant changes.
@canova canova force-pushed the source-table-improvements branch from bdcbeb6 to eeb4252 Compare March 9, 2026 13:04
@canova canova force-pushed the source-table-improvements branch from eeb4252 to 18721fd Compare March 9, 2026 13:07
@canova canova merged commit 4fe7730 into firefox-devtools:main Mar 9, 2026
19 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants