Merge upstream #4
Merged
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
This workaround was making the tests pass even when `fetch` was undefined. That made sense if you assume that the only Node-based clients of `scratch-storage` are its own tests, but it doesn't help (for example) Node-based tests in `scratch-vm`. Since we now ensure that `fetch` is polyfilled for Node, this workaround is no longer necessary.
…-in-node fix: polyfill 'fetch' for Node target
FetchTool should resolve null for the data request if the request gets a 404 response. WebHelper should properly handle this case and resolve null for the asset (instead of an asset skeleton with null 'data' field) if the asset is nowhere to be found. WebHelper should also report errors properly. BREAKING CHANGE: The WebHelper was not previously resolving with null assets even though this was the documented intended behavior of ScratchStorage.load builds on and closes scratchfoundation#363 (thanks @AndreyNikitin!), closes scratchfoundation#156, might be related to scratchfoundation#367
Add a test that FetchTool.get resolves null when it encounters a 404 for the get request
We were previously calling new Uint8Array on a promise instead of the result of the promise
…error-handling Fix fetch tool error handling
…r-tool fix(fetchworkertool): don't create Uint8Array for null response
Error objects can't be sent with `postMessage`, so report errors using strings instead.
Co-authored-by: Karishma Chadha <[email protected]>
…from-worker fix(worker): report worker errors as strings
[IBE-581] Migrate to circle
Fix tests on CI
```sh npm ci && npm i --package-lock-only && npm i --package-lock-only ```
Previously, we had Webpack's ProvidePlugin injecting `node-fetch` as a polyfill into Node builds, and web builds just relied on the `fetch` built into the browser (if it's there). Now, we use `cross-fetch` everywhere, and as a ponyfill instead of a polyfill. This broke the unit tests in `fetch-tool.js` because they were overriding `global.fetch` to mock `node-fetch`. Now, those tests use `tap.mock` to mock `cross-fetch`. Of course, `tap.mock` didn't exist in the old version of `node-tap`, so I also upgraded that.
Without this, the Headers object gets corrupted by postMessage and results in CORS errors complaining about a header field called "map"
This code will more directly follow Headers rules. In particular:
- Attempting to add metadata with a name that isn't a valid header name
will cause an error immediately instead of in `applyMetadata` (likely
at fetch time).
- Header names are not case sensitive. Using a Headers object internally
means that the metadata API will respect that. For example,
`setMetadata('a', ...)` will override an earlier value from
`setMetadata('A', ...)`. Previously, this happened "accidentally" in
`applyMetadata` instead of in the metadata container itself.
Also emphasize that the `Headers` class used in `scratchFetch.js` comes from `cross-fetch`
This should make it harder to make a mistake mocking `cross-fetch` again
Improve scratchFetch metadata and testing
…ate/pin-dependencies chore(deps): pin node orb to 5.1.0
Injecting the path into the "code" was causing the backslashes to be interpreted as escape characters instead of path separators. I could have escaped them, but this approach is more similar to what Webpack does with `arraybuffer-loader` and (IMO) easier to read.
Switch to Jest and re-enable project tests
…n-metadata Control scratchFetch with a query param
This checks for things like accidentally using the filename as the file content, etc.
…-for-gui Fix [email protected] for browser builds
AndreyNikitin
force-pushed
the
merge_upstream
branch
from
d1c9390 to
062edd2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.