This repository has been archived by the owner on Jun 24, 2024. It is now read-only.
Use fetch infrastructure to fetch source maps
#27
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nicolo-ribaudo
commented
Jan 18, 2024
source-map.bs
Outdated
| 1. Let |request| be a new [=request=] whose [=request/URL=] is |url|. | ||
| 1. [=Fetch=] |request| with [=processResponseConsumeBody=] set to the following steps given [=response=] <var ignore>response</var> and null, failure, or a [=byte sequence=] |bodyBytes|: | ||
| 1. If |bodyBytes| is null or failure, [=reject=] |promise| with a {{TypeError}} and abort these steps. | ||
| 1. Let |sourceMap| be the result of [=parsing JSON bytes to a JavaScript value=] given |bodyBytes|. |
There was a problem hiding this comment.
This is basically JSON.parse(utf8decode(bodyBytes)). There is also the option of returning an "internal spec value" rather than a JS object, but our spec is so high level that it doesn't really matter.
|
Oh I should also merge http://localhost:4507/source-map.html#json-over-http-transport in this section |
nicolo-ribaudo
commented
Jan 18, 2024
source-map.bs
Outdated
Comment on lines
439
to
441
| 1. If |url|'s [=url/scheme=] is "<code>http</code>" and |bodyBytes| [=byte sequence/starts with=] \`<code>)]}'</code>\`, then: | ||
| 1. [=While=] |bodyBytes|'s [=byte sequence/length=] is not 0 and |bodyBytes|'s 0th byte is not 0x0A (LF) or 0x0D (CR): | ||
| 1. remove the 0th byte from |bodyBytes|. |
There was a problem hiding this comment.
Do we know if this is only for HTTP or also for HTTPS?(answer by @jridgewell: yes)- Do we know what "ignore the first line" means? Some possible sets of line terminators are:
\nand\r(what HTTP defines as line terminators)\n,\r, U+2028, U+2029 (what JS defines as line terminators)
There was a problem hiding this comment.
Does anyone have bandwidth to test this in some browsers?
There was a problem hiding this comment.
According to https://developer.chrome.com/blog/sourcemaps#potential_xssi_issues, the answer could also be "just \n"
littledan
reviewed
Jan 25, 2024
littledan
approved these changes
Jan 25, 2024
gibson042
approved these changes
Jan 25, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes tc39/ecma426#30
This PR uses the
fetchspec to fetch source maps, relieving us from a couple responsibility: