Reliable JavaScript/SourceMap processing via DebugId - #81

Merged
mitsuhiko merged 5 commits into
mainfrom
sourcemap-debugid
Apr 13, 2023
Merged

Reliable JavaScript/SourceMap processing via DebugId#81
mitsuhiko merged 5 commits into
mainfrom
sourcemap-debugid

Conversation

@Swatinem

@SwatinemSwatinem commented Mar 21, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

We want to make processing / SourceMap-ing of JavaScript stack traces more reliable.
To achieve this, we want to uniquely identify a (minified / deployed) JavaScript file using a DebugId.
The same DebugId also uniquely identifies the corresponding SourceMap.
That way it should be possible to reliably look up the SourceMap corresponding to
a JavaScript file.

Rendered RFC

@SwatinemSwatinem changed the title Be able to look up SourceMaps by DebugIdReliable JavaScript/SourceMap lookup via DebugIdMar 22, 2023
@mitsuhiko

mitsuhiko commented Mar 22, 2023

Copy link
Copy Markdown
Contributor

Ad unresolved questions: I think the debug_id reference in the sourcemap is something that makes quite a bit of sense to change.

@SwatinemSwatinem changed the title Reliable JavaScript/SourceMap lookup via DebugIdReliable JavaScript/SourceMap processing via DebugIdMar 22, 2023
@Swatinem
Swatinem marked this pull request as ready for review March 23, 2023 13:17
@SwatinemSwatinem mentioned this pull request Mar 23, 2023
Closed
@iambriccardo

iambriccardo commented Mar 24, 2023

Copy link
Copy Markdown
Contributor

Good work @Swatinem on the RFC, I am a bit confused by the cons of the Add the DebugId to a global at load time. Is it a big con the parsing of the Error.stack? To me, considering the other options, it is more of a pro, unless it is computationally expensive but on this I don't know.

lforst
lforst reviewed Mar 26, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md Outdated

**cons**

- Might incur some async fetching / IO when capturing an Error. Though any `abs_path` in the stack trace should be cached already.

@lforstlforstMar 26, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This doesn't sound like a good idea to me. Mostly gut feeling but some other things come to mind:

  • Resource could be large
  • There might be delay in fetching the resource causing racing situations with navigations and so on
  • (I'll add additional reasons here when they come to mind)

Comment threadtext/0081-sourcemap-debugid.md
Comment threadtext/0081-sourcemap-debugid.md
- It does however require parsing of the `Error.stack` at time of capturing the `Error`.

An alternative implementation might use the [`import.meta.url`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import.meta)
property. This would avoid capturing and post-processing an `Error.stack`, but does require usage of ECMAScript Modules.

@lforstlforstMar 26, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

but does require usage of ECMAScript Modules

IMO this is a deal breaker. At least while IE is still somewhat relevant.

@the-spykethe-spykeMar 29, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'd rather build for the future, as when this spec become widespread the IE will be no more.

@lforstlforstMar 30, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is an implementation detail anyhow so instead of building for the future I'd rather build for compatibility right now.

@SwatinemSwatinemMar 31, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Once this is being implemented as bundler plugins, its possible to also make this configurable, with appropriate defaults. For example, if a bundler is configured to output ESM, it might inject the more compact ESM snippet, falling back to the other one in case of CommonJS/UMD output.

Comment threadtext/0081-sourcemap-debugid.md Outdated
Comment threadtext/0081-sourcemap-debugid.md
Comment threadtext/0081-sourcemap-debugid.md Outdated
Comment threadtext/0081-sourcemap-debugid.md Outdated
Comment threadtext/0081-sourcemap-debugid.md Outdated
marandaneto
marandaneto reviewed Mar 27, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md Outdated
marandaneto
marandaneto reviewed Mar 27, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
@marandaneto

marandaneto commented Mar 27, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

Have we considered languages/frameworks that are transpiled to JS? Such as Flutter Web (Dart -> JS), you don't have much control over the transpilation and minification steps, nor you do have build hooks, not sure how the inject would work here.

@Swatinem

Swatinem commented Mar 27, 2023

Copy link
Copy Markdown
ContributorAuthor

@marandaneto
I have not considered Dart / Flutter at all so far, and I do not know anything about it either. Can you give more details here? How does the compilation pipeline look like? Does Dart use SourceMaps? If so, how? Do developers usually combine that pipeline with other JS-native build tools/bundlers?

@marandaneto

marandaneto commented Mar 27, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

@marandaneto I have not considered Dart / Flutter at all so far, and I do not know anything about it either. Can you give more details here? How does the compilation pipeline look like? Does Dart use SourceMaps? If so, how? Do developers usually combine that pipeline with other JS-native build tools/bundlers?

Yeah Flutter web spits out source maps, and our symbolication works except for a few optimizations on the Dart side (as of now not a problem at all).
I guess it's possible to combine with other build tools/bundlers but it's not a common case at all, nor I have seen that, nor is documented, but maybe technically possible, I guess we don't need to support this at all at least as of now.
People just do flutter build web --source-maps and the magic happens.
Not sure which tooling flutter uses for source maps generation, would need to dig into the source code, here and here.

@the-spyke

the-spyke commented Mar 29, 2023

Copy link
Copy Markdown

Could be also hash(minifiedContent + sourceMap)

@Swatinem
Reliable JavaScript/SourceMap processing via DebugId
f9682c5
We want to make processing / SourceMap-ing of JavaScript stack traces more reliable.
To achieve this, we want to uniquely identify a (minified / deployed) JavaScript file using a DebugId.
The same DebugId also uniquely identifies the corresponding SourceMap.
That way it should be possible to _reliably_ look up the SourceMap corresponding to
a JavaScript file, which is necessary to have reliable SourceMap processing.
@lforst

lforst commented Mar 31, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

Could be also hash(minifiedContent + sourceMap)

Don't know about this, since in some cases we don't have either minifiedContent or sourceMap available. (e.g. when injecting during bundler builds)

There are ways to make the debug id deterministic, but I don't think we can or should enforce the "origin of determinism".

@iambriccardo

iambriccardo commented Apr 3, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

Considering that our goal is to capture change as defined by any change in the non-minified source, the hash of the source map is our best bet. The only caveat I am thinking of is if there is the possibility that hash(sm1) != hash(sm2) if sc1 == sc2 where sm is the source map and sc is the non-minified source. An example of this is if the bundler omits/adds some information in the source map for some reason even if the original source stays the same.

@mitsuhiko

mitsuhiko commented Apr 6, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

Auxiliary options with source hashes: Original Spec from Edge

@Swatinem

Swatinem commented Apr 6, 2023

Copy link
Copy Markdown
ContributorAuthor

Do you know if the SourceHash proposal still takes suggestions? I would recommend to be able to explicitly state the hash algorithm, similar to https://github.com/dotnet/runtime/blob/main/docs/design/specs/PE-COFF.md#pdb-checksum-debug-directory-entry-type-19 to make it forward compatible to changing the hash algorithm.
In your example SourceBundle manifest, you have prefixed the digest with the algorithm as well. And that example snippet is missing the corresponding hash for the SourceMap entry.

@mitsuhikomitsuhiko mentioned this pull request Apr 7, 2023
Open
@mitsuhiko

mitsuhiko commented Apr 7, 2023

Copy link
Copy Markdown
Contributor

@Swatinem as the source hash proposal is already implemented in Chrome I'm not sure how many changes will happen there.

I moved the source hash proposal from my comment here: #85

@mitsuhiko

mitsuhiko commented Apr 7, 2023

Copy link
Copy Markdown
Contributor

I think we generally want to merge this RFC with some minor edits, but we need to also document somewhere, what the changes to the artifact bundle are. I'm okay if this is not in the RFC as we already have it somewhere, but as it stands it's undocumented.

@mitsuhiko
mitsuhiko merged commit ab0f755 into mainApr 13, 2023
@mitsuhiko
mitsuhiko deleted the sourcemap-debugid branch April 13, 2023 08:57
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markushimarkushimarkushi left review comments
+4 more reviewers
@mitsuhikomitsuhikomitsuhiko left review comments
@the-spykethe-spykethe-spyke left review comments
@marandanetomarandanetomarandaneto left review comments
@lforstlforstlforst left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@Swatinem@mitsuhiko@iambriccardo@marandaneto@the-spyke@lforst@markushi
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Reliable JavaScript/SourceMap processing via DebugId - #81
Merged
mitsuhiko merged 5 commits into
mainfrom 
sourcemap-debugid
Apr 13, 2023
Merged
Reliable JavaScript/SourceMap processing via DebugId#81
mitsuhiko merged 5 commits into
mainfrom 
sourcemap-debugid
Conversation
@Swatinem
@SwatinemSwatinem
commented
Mar 21, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
We want to make processing / SourceMap-ing of JavaScript stack traces more reliable.

To achieve this, we want to uniquely identify a (minified / deployed) JavaScript file using a DebugId.

The same DebugId also uniquely identifies the corresponding SourceMap.

That way it should be possible to reliably look up the SourceMap corresponding to

a JavaScript file.
Rendered RFC
@SwatinemSwatinem
changed the title
Be able to look up SourceMaps by DebugIdReliable JavaScript/SourceMap lookup via DebugIdMar 22, 2023
@mitsuhiko
mitsuhiko
commented
Mar 22, 2023

Copy link


Copy Markdown

Contributor
Ad unresolved questions: I think the debug_id reference in the sourcemap is something that makes quite a bit of sense to change.
@SwatinemSwatinem
changed the title
Reliable JavaScript/SourceMap lookup via DebugIdReliable JavaScript/SourceMap processing via DebugIdMar 22, 2023
@Swatinem
Swatinem
marked this pull request as ready for review
March 23, 2023 13:17
@SwatinemSwatinem
mentioned this pull request
Mar 23, 2023

Closed

@iambriccardo
iambriccardo
commented
Mar 24, 2023

Copy link


Copy Markdown

Contributor
Good work @Swatinem on the RFC, I am a bit confused by the cons of the Add the DebugId to a global at load time. Is it a big con the parsing of the Error.stack? To me, considering the other options, it is more of a pro, unless it is computationally expensive but on this I don't know.
lforst
lforst
reviewed
Mar 26, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
Outdated


**cons**

- Might incur some async fetching / IO when capturing an Error. Though any `abs_path` in the stack trace should be cached already.
@lforstlforstMar 26, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

This doesn't sound like a good idea to me. Mostly gut feeling but some other things come to mind:
  • Resource could be large
  • There might be delay in fetching the resource causing racing situations with navigations and so on
  • (I'll add additional reasons here when they come to mind)
Comment threadtext/0081-sourcemap-debugid.md
Comment threadtext/0081-sourcemap-debugid.md
- It does however require parsing of the `Error.stack` at time of capturing the `Error`.

An alternative implementation might use the [`import.meta.url`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import.meta)
property. This would avoid capturing and post-processing an `Error.stack`, but does require usage of ECMAScript Modules.
@lforstlforstMar 26, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

but does require usage of ECMAScript Modules
IMO this is a deal breaker. At least while IE is still somewhat relevant.
@the-spykethe-spykeMar 29, 2023

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

I'd rather build for the future, as when this spec become widespread the IE will be no more.
@lforstlforstMar 30, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

This is an implementation detail anyhow so instead of building for the future I'd rather build for compatibility right now.
@SwatinemSwatinemMar 31, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Once this is being implemented as bundler plugins, its possible to also make this configurable, with appropriate defaults. For example, if a bundler is configured to output ESM, it might inject the more compact ESM snippet, falling back to the other one in case of CommonJS/UMD output.
Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Outdated

marandaneto
marandaneto
reviewed
Mar 27, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
Outdated

marandaneto
marandaneto
reviewed
Mar 27, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
@marandaneto
marandaneto
commented
Mar 27, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Have we considered languages/frameworks that are transpiled to JS? Such as Flutter Web (Dart -> JS), you don't have much control over the transpilation and minification steps, nor you do have build hooks, not sure how the inject would work here.
@Swatinem
Swatinem
commented
Mar 27, 2023

Copy link


Copy Markdown

ContributorAuthor
@marandaneto

I have not considered Dart / Flutter at all so far, and I do not know anything about it either. Can you give more details here? How does the compilation pipeline look like? Does Dart use SourceMaps? If so, how? Do developers usually combine that pipeline with other JS-native build tools/bundlers?
@marandaneto
marandaneto
commented
Mar 27, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
@marandaneto I have not considered Dart / Flutter at all so far, and I do not know anything about it either. Can you give more details here? How does the compilation pipeline look like? Does Dart use SourceMaps? If so, how? Do developers usually combine that pipeline with other JS-native build tools/bundlers?
Yeah Flutter web spits out source maps, and our symbolication works except for a few optimizations on the Dart side (as of now not a problem at all).

I guess it's possible to combine with other build tools/bundlers but it's not a common case at all, nor I have seen that, nor is documented, but maybe technically possible, I guess we don't need to support this at all at least as of now.

People just do flutter build web --source-maps and the magic happens.

Not sure which tooling flutter uses for source maps generation, would need to dig into the source code, here and here.
@the-spyke
the-spyke
commented
Mar 29, 2023

Copy link


Copy Markdown

Could be also hash(minifiedContent + sourceMap)
@Swatinem
Reliable JavaScript/SourceMap processing via DebugId
f9682c5
We want to make processing / SourceMap-ing of JavaScript stack traces more reliable.
To achieve this, we want to uniquely identify a (minified / deployed) JavaScript file using a DebugId.
The same DebugId also uniquely identifies the corresponding SourceMap.
That way it should be possible to _reliably_ look up the SourceMap corresponding to
a JavaScript file, which is necessary to have reliable SourceMap processing.
@lforst
lforst
commented
Mar 31, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Could be also hash(minifiedContent + sourceMap)
Don't know about this, since in some cases we don't have either minifiedContent or sourceMap available. (e.g. when injecting during bundler builds)
There are ways to make the debug id deterministic, but I don't think we can or should enforce the "origin of determinism".
@iambriccardo
iambriccardo
commented
Apr 3, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Considering that our goal is to capture change as defined by any change in the non-minified source, the hash of the source map is our best bet. The only caveat I am thinking of is if there is the possibility that hash(sm1) != hash(sm2) if sc1 == sc2 where sm is the source map and sc is the non-minified source. An example of this is if the bundler omits/adds some information in the source map for some reason even if the original source stays the same.
@mitsuhiko
mitsuhiko
commented
Apr 6, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Auxiliary options with source hashes: Original Spec from Edge
@Swatinem
Swatinem
commented
Apr 6, 2023

Copy link


Copy Markdown

ContributorAuthor
Do you know if the SourceHash proposal still takes suggestions? I would recommend to be able to explicitly state the hash algorithm, similar to https://github.com/dotnet/runtime/blob/main/docs/design/specs/PE-COFF.md#pdb-checksum-debug-directory-entry-type-19 to make it forward compatible to changing the hash algorithm.

In your example SourceBundle manifest, you have prefixed the digest with the algorithm as well. And that example snippet is missing the corresponding hash for the SourceMap entry.
@mitsuhikomitsuhiko
mentioned this pull request
Apr 7, 2023
 Open

@mitsuhiko
mitsuhiko
commented
Apr 7, 2023

Copy link


Copy Markdown

Contributor
@Swatinem as the source hash proposal is already implemented in Chrome I'm not sure how many changes will happen there.
I moved the source hash proposal from my comment here: #85
@mitsuhiko
mitsuhiko
commented
Apr 7, 2023

Copy link


Copy Markdown

Contributor
I think we generally want to merge this RFC with some minor edits, but we need to also document somewhere, what the changes to the artifact bundle are. I'm okay if this is not in the RFC as we already have it somewhere, but as it stands it's undocumented.
@mitsuhiko
mitsuhiko
merged commit ab0f755
into
mainApr 13, 2023
@mitsuhiko
mitsuhiko
deleted the

sourcemap-debugid

branch
April 13, 2023 08:57
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@markushimarkushimarkushi left review comments

+4 more reviewers

@mitsuhikomitsuhikomitsuhiko left review comments
@the-spykethe-spykethe-spyke left review comments
@marandanetomarandanetomarandaneto left review comments
@lforstlforstlforst left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@Swatinem@mitsuhiko@iambriccardo@marandaneto@the-spyke@lforst@markushi

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Reliable JavaScript/SourceMap processing via DebugId - #81
Merged
mitsuhiko merged 5 commits into
mainfrom 
sourcemap-debugid
Apr 13, 2023
Merged
Reliable JavaScript/SourceMap processing via DebugId#81
mitsuhiko merged 5 commits into
mainfrom 
sourcemap-debugid
Conversation
@Swatinem
@SwatinemSwatinem
commented
Mar 21, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
We want to make processing / SourceMap-ing of JavaScript stack traces more reliable.

To achieve this, we want to uniquely identify a (minified / deployed) JavaScript file using a DebugId.

The same DebugId also uniquely identifies the corresponding SourceMap.

That way it should be possible to reliably look up the SourceMap corresponding to

a JavaScript file.
Rendered RFC
@SwatinemSwatinem
changed the title
Be able to look up SourceMaps by DebugIdReliable JavaScript/SourceMap lookup via DebugIdMar 22, 2023
@mitsuhiko
mitsuhiko
commented
Mar 22, 2023

Copy link


Copy Markdown

Contributor
Ad unresolved questions: I think the debug_id reference in the sourcemap is something that makes quite a bit of sense to change.
@SwatinemSwatinem
changed the title
Reliable JavaScript/SourceMap lookup via DebugIdReliable JavaScript/SourceMap processing via DebugIdMar 22, 2023
@Swatinem
Swatinem
marked this pull request as ready for review
March 23, 2023 13:17
@SwatinemSwatinem
mentioned this pull request
Mar 23, 2023

Closed

@iambriccardo
iambriccardo
commented
Mar 24, 2023

Copy link


Copy Markdown

Contributor
Good work @Swatinem on the RFC, I am a bit confused by the cons of the Add the DebugId to a global at load time. Is it a big con the parsing of the Error.stack? To me, considering the other options, it is more of a pro, unless it is computationally expensive but on this I don't know.
lforst
lforst
reviewed
Mar 26, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
Outdated


**cons**

- Might incur some async fetching / IO when capturing an Error. Though any `abs_path` in the stack trace should be cached already.
@lforstlforstMar 26, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

This doesn't sound like a good idea to me. Mostly gut feeling but some other things come to mind:
  • Resource could be large
  • There might be delay in fetching the resource causing racing situations with navigations and so on
  • (I'll add additional reasons here when they come to mind)
Comment threadtext/0081-sourcemap-debugid.md
Comment threadtext/0081-sourcemap-debugid.md
- It does however require parsing of the `Error.stack` at time of capturing the `Error`.

An alternative implementation might use the [`import.meta.url`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import.meta)
property. This would avoid capturing and post-processing an `Error.stack`, but does require usage of ECMAScript Modules.
@lforstlforstMar 26, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

but does require usage of ECMAScript Modules
IMO this is a deal breaker. At least while IE is still somewhat relevant.
@the-spykethe-spykeMar 29, 2023

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

I'd rather build for the future, as when this spec become widespread the IE will be no more.
@lforstlforstMar 30, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

This is an implementation detail anyhow so instead of building for the future I'd rather build for compatibility right now.
@SwatinemSwatinemMar 31, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Once this is being implemented as bundler plugins, its possible to also make this configurable, with appropriate defaults. For example, if a bundler is configured to output ESM, it might inject the more compact ESM snippet, falling back to the other one in case of CommonJS/UMD output.
Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Outdated

marandaneto
marandaneto
reviewed
Mar 27, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
Outdated

marandaneto
marandaneto
reviewed
Mar 27, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
@marandaneto
marandaneto
commented
Mar 27, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Have we considered languages/frameworks that are transpiled to JS? Such as Flutter Web (Dart -> JS), you don't have much control over the transpilation and minification steps, nor you do have build hooks, not sure how the inject would work here.
@Swatinem
Swatinem
commented
Mar 27, 2023

Copy link


Copy Markdown

ContributorAuthor
@marandaneto

I have not considered Dart / Flutter at all so far, and I do not know anything about it either. Can you give more details here? How does the compilation pipeline look like? Does Dart use SourceMaps? If so, how? Do developers usually combine that pipeline with other JS-native build tools/bundlers?
@marandaneto
marandaneto
commented
Mar 27, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
@marandaneto I have not considered Dart / Flutter at all so far, and I do not know anything about it either. Can you give more details here? How does the compilation pipeline look like? Does Dart use SourceMaps? If so, how? Do developers usually combine that pipeline with other JS-native build tools/bundlers?
Yeah Flutter web spits out source maps, and our symbolication works except for a few optimizations on the Dart side (as of now not a problem at all).

I guess it's possible to combine with other build tools/bundlers but it's not a common case at all, nor I have seen that, nor is documented, but maybe technically possible, I guess we don't need to support this at all at least as of now.

People just do flutter build web --source-maps and the magic happens.

Not sure which tooling flutter uses for source maps generation, would need to dig into the source code, here and here.
@the-spyke
the-spyke
commented
Mar 29, 2023

Copy link


Copy Markdown

Could be also hash(minifiedContent + sourceMap)
@Swatinem
Reliable JavaScript/SourceMap processing via DebugId
f9682c5
We want to make processing / SourceMap-ing of JavaScript stack traces more reliable.
To achieve this, we want to uniquely identify a (minified / deployed) JavaScript file using a DebugId.
The same DebugId also uniquely identifies the corresponding SourceMap.
That way it should be possible to _reliably_ look up the SourceMap corresponding to
a JavaScript file, which is necessary to have reliable SourceMap processing.
@lforst
lforst
commented
Mar 31, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Could be also hash(minifiedContent + sourceMap)
Don't know about this, since in some cases we don't have either minifiedContent or sourceMap available. (e.g. when injecting during bundler builds)
There are ways to make the debug id deterministic, but I don't think we can or should enforce the "origin of determinism".
@iambriccardo
iambriccardo
commented
Apr 3, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Considering that our goal is to capture change as defined by any change in the non-minified source, the hash of the source map is our best bet. The only caveat I am thinking of is if there is the possibility that hash(sm1) != hash(sm2) if sc1 == sc2 where sm is the source map and sc is the non-minified source. An example of this is if the bundler omits/adds some information in the source map for some reason even if the original source stays the same.
@mitsuhiko
mitsuhiko
commented
Apr 6, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Auxiliary options with source hashes: Original Spec from Edge
@Swatinem
Swatinem
commented
Apr 6, 2023

Copy link


Copy Markdown

ContributorAuthor
Do you know if the SourceHash proposal still takes suggestions? I would recommend to be able to explicitly state the hash algorithm, similar to https://github.com/dotnet/runtime/blob/main/docs/design/specs/PE-COFF.md#pdb-checksum-debug-directory-entry-type-19 to make it forward compatible to changing the hash algorithm.

In your example SourceBundle manifest, you have prefixed the digest with the algorithm as well. And that example snippet is missing the corresponding hash for the SourceMap entry.
@mitsuhikomitsuhiko
mentioned this pull request
Apr 7, 2023
 Open

@mitsuhiko
mitsuhiko
commented
Apr 7, 2023

Copy link


Copy Markdown

Contributor
@Swatinem as the source hash proposal is already implemented in Chrome I'm not sure how many changes will happen there.
I moved the source hash proposal from my comment here: #85
@mitsuhiko
mitsuhiko
commented
Apr 7, 2023

Copy link


Copy Markdown

Contributor
I think we generally want to merge this RFC with some minor edits, but we need to also document somewhere, what the changes to the artifact bundle are. I'm okay if this is not in the RFC as we already have it somewhere, but as it stands it's undocumented.
@mitsuhiko
mitsuhiko
merged commit ab0f755
into
mainApr 13, 2023
@mitsuhiko
mitsuhiko
deleted the

sourcemap-debugid

branch
April 13, 2023 08:57
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@markushimarkushimarkushi left review comments

+4 more reviewers

@mitsuhikomitsuhikomitsuhiko left review comments
@the-spykethe-spykethe-spyke left review comments
@marandanetomarandanetomarandaneto left review comments
@lforstlforstlforst left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@Swatinem@mitsuhiko@iambriccardo@marandaneto@the-spyke@lforst@markushi

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Reliable JavaScript/SourceMap processing via DebugId - #81
Merged
mitsuhiko merged 5 commits into
mainfrom 
sourcemap-debugid
Apr 13, 2023
Merged
Reliable JavaScript/SourceMap processing via DebugId#81
mitsuhiko merged 5 commits into
mainfrom 
sourcemap-debugid
Conversation
@Swatinem
@SwatinemSwatinem
commented
Mar 21, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
We want to make processing / SourceMap-ing of JavaScript stack traces more reliable.

To achieve this, we want to uniquely identify a (minified / deployed) JavaScript file using a DebugId.

The same DebugId also uniquely identifies the corresponding SourceMap.

That way it should be possible to reliably look up the SourceMap corresponding to

a JavaScript file.
Rendered RFC
@SwatinemSwatinem
changed the title
Be able to look up SourceMaps by DebugIdReliable JavaScript/SourceMap lookup via DebugIdMar 22, 2023
@mitsuhiko
mitsuhiko
commented
Mar 22, 2023

Copy link


Copy Markdown

Contributor
Ad unresolved questions: I think the debug_id reference in the sourcemap is something that makes quite a bit of sense to change.
@SwatinemSwatinem
changed the title
Reliable JavaScript/SourceMap lookup via DebugIdReliable JavaScript/SourceMap processing via DebugIdMar 22, 2023
@Swatinem
Swatinem
marked this pull request as ready for review
March 23, 2023 13:17
@SwatinemSwatinem
mentioned this pull request
Mar 23, 2023

Closed

@iambriccardo
iambriccardo
commented
Mar 24, 2023

Copy link


Copy Markdown

Contributor
Good work @Swatinem on the RFC, I am a bit confused by the cons of the Add the DebugId to a global at load time. Is it a big con the parsing of the Error.stack? To me, considering the other options, it is more of a pro, unless it is computationally expensive but on this I don't know.
lforst
lforst
reviewed
Mar 26, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
Outdated


**cons**

- Might incur some async fetching / IO when capturing an Error. Though any `abs_path` in the stack trace should be cached already.
@lforstlforstMar 26, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

This doesn't sound like a good idea to me. Mostly gut feeling but some other things come to mind:
  • Resource could be large
  • There might be delay in fetching the resource causing racing situations with navigations and so on
  • (I'll add additional reasons here when they come to mind)
Comment threadtext/0081-sourcemap-debugid.md
Comment threadtext/0081-sourcemap-debugid.md
- It does however require parsing of the `Error.stack` at time of capturing the `Error`.

An alternative implementation might use the [`import.meta.url`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import.meta)
property. This would avoid capturing and post-processing an `Error.stack`, but does require usage of ECMAScript Modules.
@lforstlforstMar 26, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

but does require usage of ECMAScript Modules
IMO this is a deal breaker. At least while IE is still somewhat relevant.
@the-spykethe-spykeMar 29, 2023

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

I'd rather build for the future, as when this spec become widespread the IE will be no more.
@lforstlforstMar 30, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

This is an implementation detail anyhow so instead of building for the future I'd rather build for compatibility right now.
@SwatinemSwatinemMar 31, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Once this is being implemented as bundler plugins, its possible to also make this configurable, with appropriate defaults. For example, if a bundler is configured to output ESM, it might inject the more compact ESM snippet, falling back to the other one in case of CommonJS/UMD output.
Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Outdated

marandaneto
marandaneto
reviewed
Mar 27, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
Outdated

marandaneto
marandaneto
reviewed
Mar 27, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
@marandaneto
marandaneto
commented
Mar 27, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Have we considered languages/frameworks that are transpiled to JS? Such as Flutter Web (Dart -> JS), you don't have much control over the transpilation and minification steps, nor you do have build hooks, not sure how the inject would work here.
@Swatinem
Swatinem
commented
Mar 27, 2023

Copy link


Copy Markdown

ContributorAuthor
@marandaneto

I have not considered Dart / Flutter at all so far, and I do not know anything about it either. Can you give more details here? How does the compilation pipeline look like? Does Dart use SourceMaps? If so, how? Do developers usually combine that pipeline with other JS-native build tools/bundlers?
@marandaneto
marandaneto
commented
Mar 27, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
@marandaneto I have not considered Dart / Flutter at all so far, and I do not know anything about it either. Can you give more details here? How does the compilation pipeline look like? Does Dart use SourceMaps? If so, how? Do developers usually combine that pipeline with other JS-native build tools/bundlers?
Yeah Flutter web spits out source maps, and our symbolication works except for a few optimizations on the Dart side (as of now not a problem at all).

I guess it's possible to combine with other build tools/bundlers but it's not a common case at all, nor I have seen that, nor is documented, but maybe technically possible, I guess we don't need to support this at all at least as of now.

People just do flutter build web --source-maps and the magic happens.

Not sure which tooling flutter uses for source maps generation, would need to dig into the source code, here and here.
@the-spyke
the-spyke
commented
Mar 29, 2023

Copy link


Copy Markdown

Could be also hash(minifiedContent + sourceMap)
@Swatinem
Reliable JavaScript/SourceMap processing via DebugId
f9682c5
We want to make processing / SourceMap-ing of JavaScript stack traces more reliable.
To achieve this, we want to uniquely identify a (minified / deployed) JavaScript file using a DebugId.
The same DebugId also uniquely identifies the corresponding SourceMap.
That way it should be possible to _reliably_ look up the SourceMap corresponding to
a JavaScript file, which is necessary to have reliable SourceMap processing.
@lforst
lforst
commented
Mar 31, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Could be also hash(minifiedContent + sourceMap)
Don't know about this, since in some cases we don't have either minifiedContent or sourceMap available. (e.g. when injecting during bundler builds)
There are ways to make the debug id deterministic, but I don't think we can or should enforce the "origin of determinism".
@iambriccardo
iambriccardo
commented
Apr 3, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Considering that our goal is to capture change as defined by any change in the non-minified source, the hash of the source map is our best bet. The only caveat I am thinking of is if there is the possibility that hash(sm1) != hash(sm2) if sc1 == sc2 where sm is the source map and sc is the non-minified source. An example of this is if the bundler omits/adds some information in the source map for some reason even if the original source stays the same.
@mitsuhiko
mitsuhiko
commented
Apr 6, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Auxiliary options with source hashes: Original Spec from Edge
@Swatinem
Swatinem
commented
Apr 6, 2023

Copy link


Copy Markdown

ContributorAuthor
Do you know if the SourceHash proposal still takes suggestions? I would recommend to be able to explicitly state the hash algorithm, similar to https://github.com/dotnet/runtime/blob/main/docs/design/specs/PE-COFF.md#pdb-checksum-debug-directory-entry-type-19 to make it forward compatible to changing the hash algorithm.

In your example SourceBundle manifest, you have prefixed the digest with the algorithm as well. And that example snippet is missing the corresponding hash for the SourceMap entry.
@mitsuhikomitsuhiko
mentioned this pull request
Apr 7, 2023
 Open

@mitsuhiko
mitsuhiko
commented
Apr 7, 2023

Copy link


Copy Markdown

Contributor
@Swatinem as the source hash proposal is already implemented in Chrome I'm not sure how many changes will happen there.
I moved the source hash proposal from my comment here: #85
@mitsuhiko
mitsuhiko
commented
Apr 7, 2023

Copy link


Copy Markdown

Contributor
I think we generally want to merge this RFC with some minor edits, but we need to also document somewhere, what the changes to the artifact bundle are. I'm okay if this is not in the RFC as we already have it somewhere, but as it stands it's undocumented.
@mitsuhiko
mitsuhiko
merged commit ab0f755
into
mainApr 13, 2023
@mitsuhiko
mitsuhiko
deleted the

sourcemap-debugid

branch
April 13, 2023 08:57
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@markushimarkushimarkushi left review comments

+4 more reviewers

@mitsuhikomitsuhikomitsuhiko left review comments
@the-spykethe-spykethe-spyke left review comments
@marandanetomarandanetomarandaneto left review comments
@lforstlforstlforst left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@Swatinem@mitsuhiko@iambriccardo@marandaneto@the-spyke@lforst@markushi

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Reliable JavaScript/SourceMap processing via DebugId - #81
Merged
mitsuhiko merged 5 commits into
mainfrom 
sourcemap-debugid
Apr 13, 2023
Merged
Reliable JavaScript/SourceMap processing via DebugId#81
mitsuhiko merged 5 commits into
mainfrom 
sourcemap-debugid
Conversation
@Swatinem
@SwatinemSwatinem
commented
Mar 21, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
We want to make processing / SourceMap-ing of JavaScript stack traces more reliable.

To achieve this, we want to uniquely identify a (minified / deployed) JavaScript file using a DebugId.

The same DebugId also uniquely identifies the corresponding SourceMap.

That way it should be possible to reliably look up the SourceMap corresponding to

a JavaScript file.
Rendered RFC
@SwatinemSwatinem
changed the title
Be able to look up SourceMaps by DebugIdReliable JavaScript/SourceMap lookup via DebugIdMar 22, 2023
@mitsuhiko
mitsuhiko
commented
Mar 22, 2023

Copy link


Copy Markdown

Contributor
Ad unresolved questions: I think the debug_id reference in the sourcemap is something that makes quite a bit of sense to change.
@SwatinemSwatinem
changed the title
Reliable JavaScript/SourceMap lookup via DebugIdReliable JavaScript/SourceMap processing via DebugIdMar 22, 2023
@Swatinem
Swatinem
marked this pull request as ready for review
March 23, 2023 13:17
@SwatinemSwatinem
mentioned this pull request
Mar 23, 2023

Closed

@iambriccardo
iambriccardo
commented
Mar 24, 2023

Copy link


Copy Markdown

Contributor
Good work @Swatinem on the RFC, I am a bit confused by the cons of the Add the DebugId to a global at load time. Is it a big con the parsing of the Error.stack? To me, considering the other options, it is more of a pro, unless it is computationally expensive but on this I don't know.
lforst
lforst
reviewed
Mar 26, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
Outdated


**cons**

- Might incur some async fetching / IO when capturing an Error. Though any `abs_path` in the stack trace should be cached already.
@lforstlforstMar 26, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

This doesn't sound like a good idea to me. Mostly gut feeling but some other things come to mind:
  • Resource could be large
  • There might be delay in fetching the resource causing racing situations with navigations and so on
  • (I'll add additional reasons here when they come to mind)
Comment threadtext/0081-sourcemap-debugid.md
Comment threadtext/0081-sourcemap-debugid.md
- It does however require parsing of the `Error.stack` at time of capturing the `Error`.

An alternative implementation might use the [`import.meta.url`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import.meta)
property. This would avoid capturing and post-processing an `Error.stack`, but does require usage of ECMAScript Modules.
@lforstlforstMar 26, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

but does require usage of ECMAScript Modules
IMO this is a deal breaker. At least while IE is still somewhat relevant.
@the-spykethe-spykeMar 29, 2023

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

I'd rather build for the future, as when this spec become widespread the IE will be no more.
@lforstlforstMar 30, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

This is an implementation detail anyhow so instead of building for the future I'd rather build for compatibility right now.
@SwatinemSwatinemMar 31, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Once this is being implemented as bundler plugins, its possible to also make this configurable, with appropriate defaults. For example, if a bundler is configured to output ESM, it might inject the more compact ESM snippet, falling back to the other one in case of CommonJS/UMD output.
Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Outdated

marandaneto
marandaneto
reviewed
Mar 27, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
Outdated

marandaneto
marandaneto
reviewed
Mar 27, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
@marandaneto
marandaneto
commented
Mar 27, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Have we considered languages/frameworks that are transpiled to JS? Such as Flutter Web (Dart -> JS), you don't have much control over the transpilation and minification steps, nor you do have build hooks, not sure how the inject would work here.
@Swatinem
Swatinem
commented
Mar 27, 2023

Copy link


Copy Markdown

ContributorAuthor
@marandaneto

I have not considered Dart / Flutter at all so far, and I do not know anything about it either. Can you give more details here? How does the compilation pipeline look like? Does Dart use SourceMaps? If so, how? Do developers usually combine that pipeline with other JS-native build tools/bundlers?
@marandaneto
marandaneto
commented
Mar 27, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
@marandaneto I have not considered Dart / Flutter at all so far, and I do not know anything about it either. Can you give more details here? How does the compilation pipeline look like? Does Dart use SourceMaps? If so, how? Do developers usually combine that pipeline with other JS-native build tools/bundlers?
Yeah Flutter web spits out source maps, and our symbolication works except for a few optimizations on the Dart side (as of now not a problem at all).

I guess it's possible to combine with other build tools/bundlers but it's not a common case at all, nor I have seen that, nor is documented, but maybe technically possible, I guess we don't need to support this at all at least as of now.

People just do flutter build web --source-maps and the magic happens.

Not sure which tooling flutter uses for source maps generation, would need to dig into the source code, here and here.
@the-spyke
the-spyke
commented
Mar 29, 2023

Copy link


Copy Markdown

Could be also hash(minifiedContent + sourceMap)
@Swatinem
Reliable JavaScript/SourceMap processing via DebugId
f9682c5
We want to make processing / SourceMap-ing of JavaScript stack traces more reliable.
To achieve this, we want to uniquely identify a (minified / deployed) JavaScript file using a DebugId.
The same DebugId also uniquely identifies the corresponding SourceMap.
That way it should be possible to _reliably_ look up the SourceMap corresponding to
a JavaScript file, which is necessary to have reliable SourceMap processing.
@lforst
lforst
commented
Mar 31, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Could be also hash(minifiedContent + sourceMap)
Don't know about this, since in some cases we don't have either minifiedContent or sourceMap available. (e.g. when injecting during bundler builds)
There are ways to make the debug id deterministic, but I don't think we can or should enforce the "origin of determinism".
@iambriccardo
iambriccardo
commented
Apr 3, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Considering that our goal is to capture change as defined by any change in the non-minified source, the hash of the source map is our best bet. The only caveat I am thinking of is if there is the possibility that hash(sm1) != hash(sm2) if sc1 == sc2 where sm is the source map and sc is the non-minified source. An example of this is if the bundler omits/adds some information in the source map for some reason even if the original source stays the same.
@mitsuhiko
mitsuhiko
commented
Apr 6, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Auxiliary options with source hashes: Original Spec from Edge
@Swatinem
Swatinem
commented
Apr 6, 2023

Copy link


Copy Markdown

ContributorAuthor
Do you know if the SourceHash proposal still takes suggestions? I would recommend to be able to explicitly state the hash algorithm, similar to https://github.com/dotnet/runtime/blob/main/docs/design/specs/PE-COFF.md#pdb-checksum-debug-directory-entry-type-19 to make it forward compatible to changing the hash algorithm.

In your example SourceBundle manifest, you have prefixed the digest with the algorithm as well. And that example snippet is missing the corresponding hash for the SourceMap entry.
@mitsuhikomitsuhiko
mentioned this pull request
Apr 7, 2023
 Open

@mitsuhiko
mitsuhiko
commented
Apr 7, 2023

Copy link


Copy Markdown

Contributor
@Swatinem as the source hash proposal is already implemented in Chrome I'm not sure how many changes will happen there.
I moved the source hash proposal from my comment here: #85
@mitsuhiko
mitsuhiko
commented
Apr 7, 2023

Copy link


Copy Markdown

Contributor
I think we generally want to merge this RFC with some minor edits, but we need to also document somewhere, what the changes to the artifact bundle are. I'm okay if this is not in the RFC as we already have it somewhere, but as it stands it's undocumented.
@mitsuhiko
mitsuhiko
merged commit ab0f755
into
mainApr 13, 2023
@mitsuhiko
mitsuhiko
deleted the

sourcemap-debugid

branch
April 13, 2023 08:57
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@markushimarkushimarkushi left review comments

+4 more reviewers

@mitsuhikomitsuhikomitsuhiko left review comments
@the-spykethe-spykethe-spyke left review comments
@marandanetomarandanetomarandaneto left review comments
@lforstlforstlforst left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@Swatinem@mitsuhiko@iambriccardo@marandaneto@the-spyke@lforst@markushi

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Reliable JavaScript/SourceMap processing via DebugId - #81
Merged
mitsuhiko merged 5 commits into
mainfrom 
sourcemap-debugid
Apr 13, 2023
Merged
Reliable JavaScript/SourceMap processing via DebugId#81
mitsuhiko merged 5 commits into
mainfrom 
sourcemap-debugid
Conversation
@Swatinem
@SwatinemSwatinem
commented
Mar 21, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
We want to make processing / SourceMap-ing of JavaScript stack traces more reliable.

To achieve this, we want to uniquely identify a (minified / deployed) JavaScript file using a DebugId.

The same DebugId also uniquely identifies the corresponding SourceMap.

That way it should be possible to reliably look up the SourceMap corresponding to

a JavaScript file.
Rendered RFC
@SwatinemSwatinem
changed the title
Be able to look up SourceMaps by DebugIdReliable JavaScript/SourceMap lookup via DebugIdMar 22, 2023
@mitsuhiko
mitsuhiko
commented
Mar 22, 2023

Copy link


Copy Markdown

Contributor
Ad unresolved questions: I think the debug_id reference in the sourcemap is something that makes quite a bit of sense to change.
@SwatinemSwatinem
changed the title
Reliable JavaScript/SourceMap lookup via DebugIdReliable JavaScript/SourceMap processing via DebugIdMar 22, 2023
@Swatinem
Swatinem
marked this pull request as ready for review
March 23, 2023 13:17
@SwatinemSwatinem
mentioned this pull request
Mar 23, 2023

Closed

@iambriccardo
iambriccardo
commented
Mar 24, 2023

Copy link


Copy Markdown

Contributor
Good work @Swatinem on the RFC, I am a bit confused by the cons of the Add the DebugId to a global at load time. Is it a big con the parsing of the Error.stack? To me, considering the other options, it is more of a pro, unless it is computationally expensive but on this I don't know.
lforst
lforst
reviewed
Mar 26, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
Outdated


**cons**

- Might incur some async fetching / IO when capturing an Error. Though any `abs_path` in the stack trace should be cached already.
@lforstlforstMar 26, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

This doesn't sound like a good idea to me. Mostly gut feeling but some other things come to mind:
  • Resource could be large
  • There might be delay in fetching the resource causing racing situations with navigations and so on
  • (I'll add additional reasons here when they come to mind)
Comment threadtext/0081-sourcemap-debugid.md
Comment threadtext/0081-sourcemap-debugid.md
- It does however require parsing of the `Error.stack` at time of capturing the `Error`.

An alternative implementation might use the [`import.meta.url`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import.meta)
property. This would avoid capturing and post-processing an `Error.stack`, but does require usage of ECMAScript Modules.
@lforstlforstMar 26, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

but does require usage of ECMAScript Modules
IMO this is a deal breaker. At least while IE is still somewhat relevant.
@the-spykethe-spykeMar 29, 2023

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

I'd rather build for the future, as when this spec become widespread the IE will be no more.
@lforstlforstMar 30, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

This is an implementation detail anyhow so instead of building for the future I'd rather build for compatibility right now.
@SwatinemSwatinemMar 31, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Once this is being implemented as bundler plugins, its possible to also make this configurable, with appropriate defaults. For example, if a bundler is configured to output ESM, it might inject the more compact ESM snippet, falling back to the other one in case of CommonJS/UMD output.
Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Outdated

marandaneto
marandaneto
reviewed
Mar 27, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
Outdated

marandaneto
marandaneto
reviewed
Mar 27, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
@marandaneto
marandaneto
commented
Mar 27, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Have we considered languages/frameworks that are transpiled to JS? Such as Flutter Web (Dart -> JS), you don't have much control over the transpilation and minification steps, nor you do have build hooks, not sure how the inject would work here.
@Swatinem
Swatinem
commented
Mar 27, 2023

Copy link


Copy Markdown

ContributorAuthor
@marandaneto

I have not considered Dart / Flutter at all so far, and I do not know anything about it either. Can you give more details here? How does the compilation pipeline look like? Does Dart use SourceMaps? If so, how? Do developers usually combine that pipeline with other JS-native build tools/bundlers?
@marandaneto
marandaneto
commented
Mar 27, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
@marandaneto I have not considered Dart / Flutter at all so far, and I do not know anything about it either. Can you give more details here? How does the compilation pipeline look like? Does Dart use SourceMaps? If so, how? Do developers usually combine that pipeline with other JS-native build tools/bundlers?
Yeah Flutter web spits out source maps, and our symbolication works except for a few optimizations on the Dart side (as of now not a problem at all).

I guess it's possible to combine with other build tools/bundlers but it's not a common case at all, nor I have seen that, nor is documented, but maybe technically possible, I guess we don't need to support this at all at least as of now.

People just do flutter build web --source-maps and the magic happens.

Not sure which tooling flutter uses for source maps generation, would need to dig into the source code, here and here.
@the-spyke
the-spyke
commented
Mar 29, 2023

Copy link


Copy Markdown

Could be also hash(minifiedContent + sourceMap)
@Swatinem
Reliable JavaScript/SourceMap processing via DebugId
f9682c5
We want to make processing / SourceMap-ing of JavaScript stack traces more reliable.
To achieve this, we want to uniquely identify a (minified / deployed) JavaScript file using a DebugId.
The same DebugId also uniquely identifies the corresponding SourceMap.
That way it should be possible to _reliably_ look up the SourceMap corresponding to
a JavaScript file, which is necessary to have reliable SourceMap processing.
@lforst
lforst
commented
Mar 31, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Could be also hash(minifiedContent + sourceMap)
Don't know about this, since in some cases we don't have either minifiedContent or sourceMap available. (e.g. when injecting during bundler builds)
There are ways to make the debug id deterministic, but I don't think we can or should enforce the "origin of determinism".
@iambriccardo
iambriccardo
commented
Apr 3, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Considering that our goal is to capture change as defined by any change in the non-minified source, the hash of the source map is our best bet. The only caveat I am thinking of is if there is the possibility that hash(sm1) != hash(sm2) if sc1 == sc2 where sm is the source map and sc is the non-minified source. An example of this is if the bundler omits/adds some information in the source map for some reason even if the original source stays the same.
@mitsuhiko
mitsuhiko
commented
Apr 6, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Auxiliary options with source hashes: Original Spec from Edge
@Swatinem
Swatinem
commented
Apr 6, 2023

Copy link


Copy Markdown

ContributorAuthor
Do you know if the SourceHash proposal still takes suggestions? I would recommend to be able to explicitly state the hash algorithm, similar to https://github.com/dotnet/runtime/blob/main/docs/design/specs/PE-COFF.md#pdb-checksum-debug-directory-entry-type-19 to make it forward compatible to changing the hash algorithm.

In your example SourceBundle manifest, you have prefixed the digest with the algorithm as well. And that example snippet is missing the corresponding hash for the SourceMap entry.
@mitsuhikomitsuhiko
mentioned this pull request
Apr 7, 2023
 Open

@mitsuhiko
mitsuhiko
commented
Apr 7, 2023

Copy link


Copy Markdown

Contributor
@Swatinem as the source hash proposal is already implemented in Chrome I'm not sure how many changes will happen there.
I moved the source hash proposal from my comment here: #85
@mitsuhiko
mitsuhiko
commented
Apr 7, 2023

Copy link


Copy Markdown

Contributor
I think we generally want to merge this RFC with some minor edits, but we need to also document somewhere, what the changes to the artifact bundle are. I'm okay if this is not in the RFC as we already have it somewhere, but as it stands it's undocumented.
@mitsuhiko
mitsuhiko
merged commit ab0f755
into
mainApr 13, 2023
@mitsuhiko
mitsuhiko
deleted the

sourcemap-debugid

branch
April 13, 2023 08:57
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@markushimarkushimarkushi left review comments

+4 more reviewers

@mitsuhikomitsuhikomitsuhiko left review comments
@the-spykethe-spykethe-spyke left review comments
@marandanetomarandanetomarandaneto left review comments
@lforstlforstlforst left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@Swatinem@mitsuhiko@iambriccardo@marandaneto@the-spyke@lforst@markushi

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Reliable JavaScript/SourceMap processing via DebugId - #81
Merged
mitsuhiko merged 5 commits into
mainfrom 
sourcemap-debugid
Apr 13, 2023
Merged
Reliable JavaScript/SourceMap processing via DebugId#81
mitsuhiko merged 5 commits into
mainfrom 
sourcemap-debugid
Conversation
@Swatinem
@SwatinemSwatinem
commented
Mar 21, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
We want to make processing / SourceMap-ing of JavaScript stack traces more reliable.

To achieve this, we want to uniquely identify a (minified / deployed) JavaScript file using a DebugId.

The same DebugId also uniquely identifies the corresponding SourceMap.

That way it should be possible to reliably look up the SourceMap corresponding to

a JavaScript file.
Rendered RFC
@SwatinemSwatinem
changed the title
Be able to look up SourceMaps by DebugIdReliable JavaScript/SourceMap lookup via DebugIdMar 22, 2023
@mitsuhiko
mitsuhiko
commented
Mar 22, 2023

Copy link


Copy Markdown

Contributor
Ad unresolved questions: I think the debug_id reference in the sourcemap is something that makes quite a bit of sense to change.
@SwatinemSwatinem
changed the title
Reliable JavaScript/SourceMap lookup via DebugIdReliable JavaScript/SourceMap processing via DebugIdMar 22, 2023
@Swatinem
Swatinem
marked this pull request as ready for review
March 23, 2023 13:17
@SwatinemSwatinem
mentioned this pull request
Mar 23, 2023

Closed

@iambriccardo
iambriccardo
commented
Mar 24, 2023

Copy link


Copy Markdown

Contributor
Good work @Swatinem on the RFC, I am a bit confused by the cons of the Add the DebugId to a global at load time. Is it a big con the parsing of the Error.stack? To me, considering the other options, it is more of a pro, unless it is computationally expensive but on this I don't know.
lforst
lforst
reviewed
Mar 26, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
Outdated


**cons**

- Might incur some async fetching / IO when capturing an Error. Though any `abs_path` in the stack trace should be cached already.
@lforstlforstMar 26, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

This doesn't sound like a good idea to me. Mostly gut feeling but some other things come to mind:
  • Resource could be large
  • There might be delay in fetching the resource causing racing situations with navigations and so on
  • (I'll add additional reasons here when they come to mind)
Comment threadtext/0081-sourcemap-debugid.md
Comment threadtext/0081-sourcemap-debugid.md
- It does however require parsing of the `Error.stack` at time of capturing the `Error`.

An alternative implementation might use the [`import.meta.url`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import.meta)
property. This would avoid capturing and post-processing an `Error.stack`, but does require usage of ECMAScript Modules.
@lforstlforstMar 26, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

but does require usage of ECMAScript Modules
IMO this is a deal breaker. At least while IE is still somewhat relevant.
@the-spykethe-spykeMar 29, 2023

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

I'd rather build for the future, as when this spec become widespread the IE will be no more.
@lforstlforstMar 30, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

This is an implementation detail anyhow so instead of building for the future I'd rather build for compatibility right now.
@SwatinemSwatinemMar 31, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Once this is being implemented as bundler plugins, its possible to also make this configurable, with appropriate defaults. For example, if a bundler is configured to output ESM, it might inject the more compact ESM snippet, falling back to the other one in case of CommonJS/UMD output.
Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Outdated

marandaneto
marandaneto
reviewed
Mar 27, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
Outdated

marandaneto
marandaneto
reviewed
Mar 27, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
@marandaneto
marandaneto
commented
Mar 27, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Have we considered languages/frameworks that are transpiled to JS? Such as Flutter Web (Dart -> JS), you don't have much control over the transpilation and minification steps, nor you do have build hooks, not sure how the inject would work here.
@Swatinem
Swatinem
commented
Mar 27, 2023

Copy link


Copy Markdown

ContributorAuthor
@marandaneto

I have not considered Dart / Flutter at all so far, and I do not know anything about it either. Can you give more details here? How does the compilation pipeline look like? Does Dart use SourceMaps? If so, how? Do developers usually combine that pipeline with other JS-native build tools/bundlers?
@marandaneto
marandaneto
commented
Mar 27, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
@marandaneto I have not considered Dart / Flutter at all so far, and I do not know anything about it either. Can you give more details here? How does the compilation pipeline look like? Does Dart use SourceMaps? If so, how? Do developers usually combine that pipeline with other JS-native build tools/bundlers?
Yeah Flutter web spits out source maps, and our symbolication works except for a few optimizations on the Dart side (as of now not a problem at all).

I guess it's possible to combine with other build tools/bundlers but it's not a common case at all, nor I have seen that, nor is documented, but maybe technically possible, I guess we don't need to support this at all at least as of now.

People just do flutter build web --source-maps and the magic happens.

Not sure which tooling flutter uses for source maps generation, would need to dig into the source code, here and here.
@the-spyke
the-spyke
commented
Mar 29, 2023

Copy link


Copy Markdown

Could be also hash(minifiedContent + sourceMap)
@Swatinem
Reliable JavaScript/SourceMap processing via DebugId
f9682c5
We want to make processing / SourceMap-ing of JavaScript stack traces more reliable.
To achieve this, we want to uniquely identify a (minified / deployed) JavaScript file using a DebugId.
The same DebugId also uniquely identifies the corresponding SourceMap.
That way it should be possible to _reliably_ look up the SourceMap corresponding to
a JavaScript file, which is necessary to have reliable SourceMap processing.
@lforst
lforst
commented
Mar 31, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Could be also hash(minifiedContent + sourceMap)
Don't know about this, since in some cases we don't have either minifiedContent or sourceMap available. (e.g. when injecting during bundler builds)
There are ways to make the debug id deterministic, but I don't think we can or should enforce the "origin of determinism".
@iambriccardo
iambriccardo
commented
Apr 3, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Considering that our goal is to capture change as defined by any change in the non-minified source, the hash of the source map is our best bet. The only caveat I am thinking of is if there is the possibility that hash(sm1) != hash(sm2) if sc1 == sc2 where sm is the source map and sc is the non-minified source. An example of this is if the bundler omits/adds some information in the source map for some reason even if the original source stays the same.
@mitsuhiko
mitsuhiko
commented
Apr 6, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Auxiliary options with source hashes: Original Spec from Edge
@Swatinem
Swatinem
commented
Apr 6, 2023

Copy link


Copy Markdown

ContributorAuthor
Do you know if the SourceHash proposal still takes suggestions? I would recommend to be able to explicitly state the hash algorithm, similar to https://github.com/dotnet/runtime/blob/main/docs/design/specs/PE-COFF.md#pdb-checksum-debug-directory-entry-type-19 to make it forward compatible to changing the hash algorithm.

In your example SourceBundle manifest, you have prefixed the digest with the algorithm as well. And that example snippet is missing the corresponding hash for the SourceMap entry.
@mitsuhikomitsuhiko
mentioned this pull request
Apr 7, 2023
 Open

@mitsuhiko
mitsuhiko
commented
Apr 7, 2023

Copy link


Copy Markdown

Contributor
@Swatinem as the source hash proposal is already implemented in Chrome I'm not sure how many changes will happen there.
I moved the source hash proposal from my comment here: #85
@mitsuhiko
mitsuhiko
commented
Apr 7, 2023

Copy link


Copy Markdown

Contributor
I think we generally want to merge this RFC with some minor edits, but we need to also document somewhere, what the changes to the artifact bundle are. I'm okay if this is not in the RFC as we already have it somewhere, but as it stands it's undocumented.
@mitsuhiko
mitsuhiko
merged commit ab0f755
into
mainApr 13, 2023
@mitsuhiko
mitsuhiko
deleted the

sourcemap-debugid

branch
April 13, 2023 08:57
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@markushimarkushimarkushi left review comments

+4 more reviewers

@mitsuhikomitsuhikomitsuhiko left review comments
@the-spykethe-spykethe-spyke left review comments
@marandanetomarandanetomarandaneto left review comments
@lforstlforstlforst left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@Swatinem@mitsuhiko@iambriccardo@marandaneto@the-spyke@lforst@markushi

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Reliable JavaScript/SourceMap processing via DebugId - #81
Merged
mitsuhiko merged 5 commits into
mainfrom 
sourcemap-debugid
Apr 13, 2023
Merged
Reliable JavaScript/SourceMap processing via DebugId#81
mitsuhiko merged 5 commits into
mainfrom 
sourcemap-debugid
Conversation
@Swatinem
@SwatinemSwatinem
commented
Mar 21, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
We want to make processing / SourceMap-ing of JavaScript stack traces more reliable.

To achieve this, we want to uniquely identify a (minified / deployed) JavaScript file using a DebugId.

The same DebugId also uniquely identifies the corresponding SourceMap.

That way it should be possible to reliably look up the SourceMap corresponding to

a JavaScript file.
Rendered RFC
@SwatinemSwatinem
changed the title
Be able to look up SourceMaps by DebugIdReliable JavaScript/SourceMap lookup via DebugIdMar 22, 2023
@mitsuhiko
mitsuhiko
commented
Mar 22, 2023

Copy link


Copy Markdown

Contributor
Ad unresolved questions: I think the debug_id reference in the sourcemap is something that makes quite a bit of sense to change.
@SwatinemSwatinem
changed the title
Reliable JavaScript/SourceMap lookup via DebugIdReliable JavaScript/SourceMap processing via DebugIdMar 22, 2023
@Swatinem
Swatinem
marked this pull request as ready for review
March 23, 2023 13:17
@SwatinemSwatinem
mentioned this pull request
Mar 23, 2023

Closed

@iambriccardo
iambriccardo
commented
Mar 24, 2023

Copy link


Copy Markdown

Contributor
Good work @Swatinem on the RFC, I am a bit confused by the cons of the Add the DebugId to a global at load time. Is it a big con the parsing of the Error.stack? To me, considering the other options, it is more of a pro, unless it is computationally expensive but on this I don't know.
lforst
lforst
reviewed
Mar 26, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
Outdated


**cons**

- Might incur some async fetching / IO when capturing an Error. Though any `abs_path` in the stack trace should be cached already.
@lforstlforstMar 26, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

This doesn't sound like a good idea to me. Mostly gut feeling but some other things come to mind:
  • Resource could be large
  • There might be delay in fetching the resource causing racing situations with navigations and so on
  • (I'll add additional reasons here when they come to mind)
Comment threadtext/0081-sourcemap-debugid.md
Comment threadtext/0081-sourcemap-debugid.md
- It does however require parsing of the `Error.stack` at time of capturing the `Error`.

An alternative implementation might use the [`import.meta.url`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import.meta)
property. This would avoid capturing and post-processing an `Error.stack`, but does require usage of ECMAScript Modules.
@lforstlforstMar 26, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

but does require usage of ECMAScript Modules
IMO this is a deal breaker. At least while IE is still somewhat relevant.
@the-spykethe-spykeMar 29, 2023

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

I'd rather build for the future, as when this spec become widespread the IE will be no more.
@lforstlforstMar 30, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

This is an implementation detail anyhow so instead of building for the future I'd rather build for compatibility right now.
@SwatinemSwatinemMar 31, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Once this is being implemented as bundler plugins, its possible to also make this configurable, with appropriate defaults. For example, if a bundler is configured to output ESM, it might inject the more compact ESM snippet, falling back to the other one in case of CommonJS/UMD output.
Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Outdated

Comment threadtext/0081-sourcemap-debugid.md
Outdated

marandaneto
marandaneto
reviewed
Mar 27, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
Outdated

marandaneto
marandaneto
reviewed
Mar 27, 2023
View reviewed changes
Comment threadtext/0081-sourcemap-debugid.md
@marandaneto
marandaneto
commented
Mar 27, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Have we considered languages/frameworks that are transpiled to JS? Such as Flutter Web (Dart -> JS), you don't have much control over the transpilation and minification steps, nor you do have build hooks, not sure how the inject would work here.
@Swatinem
Swatinem
commented
Mar 27, 2023

Copy link


Copy Markdown

ContributorAuthor
@marandaneto

I have not considered Dart / Flutter at all so far, and I do not know anything about it either. Can you give more details here? How does the compilation pipeline look like? Does Dart use SourceMaps? If so, how? Do developers usually combine that pipeline with other JS-native build tools/bundlers?
@marandaneto
marandaneto
commented
Mar 27, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
@marandaneto I have not considered Dart / Flutter at all so far, and I do not know anything about it either. Can you give more details here? How does the compilation pipeline look like? Does Dart use SourceMaps? If so, how? Do developers usually combine that pipeline with other JS-native build tools/bundlers?
Yeah Flutter web spits out source maps, and our symbolication works except for a few optimizations on the Dart side (as of now not a problem at all).

I guess it's possible to combine with other build tools/bundlers but it's not a common case at all, nor I have seen that, nor is documented, but maybe technically possible, I guess we don't need to support this at all at least as of now.

People just do flutter build web --source-maps and the magic happens.

Not sure which tooling flutter uses for source maps generation, would need to dig into the source code, here and here.
@the-spyke
the-spyke
commented
Mar 29, 2023

Copy link


Copy Markdown

Could be also hash(minifiedContent + sourceMap)
@Swatinem
Reliable JavaScript/SourceMap processing via DebugId
f9682c5
We want to make processing / SourceMap-ing of JavaScript stack traces more reliable.
To achieve this, we want to uniquely identify a (minified / deployed) JavaScript file using a DebugId.
The same DebugId also uniquely identifies the corresponding SourceMap.
That way it should be possible to _reliably_ look up the SourceMap corresponding to
a JavaScript file, which is necessary to have reliable SourceMap processing.
@lforst
lforst
commented
Mar 31, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Could be also hash(minifiedContent + sourceMap)
Don't know about this, since in some cases we don't have either minifiedContent or sourceMap available. (e.g. when injecting during bundler builds)
There are ways to make the debug id deterministic, but I don't think we can or should enforce the "origin of determinism".
@iambriccardo
iambriccardo
commented
Apr 3, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Considering that our goal is to capture change as defined by any change in the non-minified source, the hash of the source map is our best bet. The only caveat I am thinking of is if there is the possibility that hash(sm1) != hash(sm2) if sc1 == sc2 where sm is the source map and sc is the non-minified source. An example of this is if the bundler omits/adds some information in the source map for some reason even if the original source stays the same.
@mitsuhiko
mitsuhiko
commented
Apr 6, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Auxiliary options with source hashes: Original Spec from Edge
@Swatinem
Swatinem
commented
Apr 6, 2023

Copy link


Copy Markdown

ContributorAuthor
Do you know if the SourceHash proposal still takes suggestions? I would recommend to be able to explicitly state the hash algorithm, similar to https://github.com/dotnet/runtime/blob/main/docs/design/specs/PE-COFF.md#pdb-checksum-debug-directory-entry-type-19 to make it forward compatible to changing the hash algorithm.

In your example SourceBundle manifest, you have prefixed the digest with the algorithm as well. And that example snippet is missing the corresponding hash for the SourceMap entry.
@mitsuhikomitsuhiko
mentioned this pull request
Apr 7, 2023
 Open

@mitsuhiko
mitsuhiko
commented
Apr 7, 2023

Copy link


Copy Markdown

Contributor
@Swatinem as the source hash proposal is already implemented in Chrome I'm not sure how many changes will happen there.
I moved the source hash proposal from my comment here: #85
@mitsuhiko
mitsuhiko
commented
Apr 7, 2023

Copy link


Copy Markdown

Contributor
I think we generally want to merge this RFC with some minor edits, but we need to also document somewhere, what the changes to the artifact bundle are. I'm okay if this is not in the RFC as we already have it somewhere, but as it stands it's undocumented.
@mitsuhiko
mitsuhiko
merged commit ab0f755
into
mainApr 13, 2023
@mitsuhiko
mitsuhiko
deleted the

sourcemap-debugid

branch
April 13, 2023 08:57
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@markushimarkushimarkushi left review comments

+4 more reviewers

@mitsuhikomitsuhikomitsuhiko left review comments
@the-spykethe-spykethe-spyke left review comments
@marandanetomarandanetomarandaneto left review comments
@lforstlforstlforst left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@Swatinem@mitsuhiko@iambriccardo@marandaneto@the-spyke@lforst@markushi