-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core: gather source maps #9101
core: gather source maps #9101
Conversation
WIP b/c no tests, and: I want to put source maps in a new artifact, because gathering them is a lot of network requests and parsing that should only be done if an audit explicitly needs it - which, right now, does not exist. It could be in a new gatherer, but I'd like to use the Is there a nice way to pass the driver to a computed artifact? Or is it better to forgo |
Sadly, no. Computed artifacts are firmly in audit phase territory and there's no way to have dependencies between artifacts unless one is a
I'm not sure I understand the decision here, since it sounds like if you needed a separate gatherer either way you could have the separate one listen on |
Failed to realize it's not required to do that in the |
const a = 1; | ||
// Use this obnoxious URL because using a non-existent url from localhost will return a | ||
// bunch of HTML, which will be parsed, and no fetch error occurs. | ||
//# sourceMappingURL=http://www.this-will-not-exist-blah-go-rockets.com/some-map.js.map |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess i could do http://localhost:1234
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
go-rockets
😱
haha 😉 to your actual point though, :1234
is the default for parcel
which I tend to have running so that might cause something unexpected too ;) haha
how about an entirely invalid URL? or is that then testing something different you're not concerned about?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i suppose anything that results in a fetch error will work, including a malformed URL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
go-rockets
Did that just to get a rise out of you ;)
Codecov Report
@@ Coverage Diff @@
## master #9101 +/- ##
==========================================
- Coverage 91.44% 91.43% -0.02%
==========================================
Files 291 292 +1
Lines 9929 9981 +52
==========================================
+ Hits 9080 9126 +46
- Misses 849 855 +6
Continue to review full report at Codecov.
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
it seems half of these comments could be @googlebot 👎 |
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
@brendankenny @exterkamp ya'll wanna add your personal emails to the CLA thing? click the Googler link above |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗺 🚜
LGTM!
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
#9097
Gather source maps.
Nothing done with them yet.