Introduce sourcemapIgnoreList config predicate
#4848
Conversation
✅ Deploy Preview for rollupjs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
bmeurer
changed the title
sourcemapIgnoreList config predicate
bmeurer
force-pushed
the
feat/x_google_ignoreList
branch
from
e997865 to
d768a18
Compare
|
I rebased this on the |
Codecov Report
@@ Coverage Diff @@
## master #4848 +/- ##
==========================================
- Coverage 98.99% 98.98% -0.02%
==========================================
Files 219 219
Lines 7888 7898 +10
Branches 2184 2189 +5
==========================================
+ Hits 7809 7818 +9
Misses 26 26
- Partials 53 54 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
bmeurer
force-pushed
the
feat/x_google_ignoreList
branch
from
d768a18 to
3c6f36a
Compare
There was a problem hiding this comment.
Great stuff! Especially considering there was virtually not feedback loop you picked up nicely how to write Rollup tests etc ;)
Only minor point for me is that the tests never look at the second argument of the sourcemapIgnoreList function, and only vaguely at the first one. Considering these are direct parts of the API, it would be nice at least at one point to assert what is passed here to avoid breaking changes if we should ever refactor here.
Otherwise, you got nearly 100% coverage :) The only thing missing is the else case for if (!map.x_google_ignoreList.includes(sourcesIndex)) {, but if I understand correctly, this is probably close to impossible to encounter while on the other side it would be foolish not to handle this case explicitly to avoid future regressions. So this is fine from my side.
| } | ||
| } | ||
| map.sources[sourcesIndex] = normalize(sourcePath); | ||
| } |
There was a problem hiding this comment.
That looks like it should definitely not hurt performance, thanks a lot!
bmeurer
force-pushed
the
feat/x_google_ignoreList
branch
from
3c6f36a to
4595731
Compare
I added a trivial test case that also sanity checks the second parameter.
I'm planning to test this, once rollup preserves |
bmeurer
force-pushed
the
feat/x_google_ignoreList
branch
from
4595731 to
071c272
Compare
This adds a new `sourcemapIgnoreList` config predicate that works similar to the existing `sourcemapPathTransform`, and can be provided to mark certain source files as ignore-listed when generating source maps. This is useful for libraries and frameworks that provide code that is not relevant to the debugging experience of the customer. Chrome DevTools already supports this new `x_google_ignoreList` field in the source maps[^1]. Doc: https://goo.gle/devtools-ignoreList-adoption [^1]: https://developer.chrome.com/blog/devtools-better-angular-debugging
bmeurer
force-pushed
the
feat/x_google_ignoreList
branch
from
071c272 to
9c3eb9a
Compare
|
This PR has been released as part of rollup@3.16.0. You can test it via |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
#4847
Description
This PR updates rollup to leverage the initial
x_google_ignoreListsupport in magic-string, and introduces asourcemapIgnoreListconfig predicate that works similar to the existingsourcemapPathTransform, and can be provided to mark certain source files as ignore-listed when generating source maps.This is useful for libraries and frameworks that provide code that is not relevant to the debugging experience of the customer. Chrome DevTools already supports this new
x_google_ignoreListfield in the source maps1.Doc: https://goo.gle/devtools-ignoreList-adoption
Footnotes
https://developer.chrome.com/blog/devtools-better-angular-debugging ↩