Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
* fix: polyfill 'fetch' for Node target * fix(tests): remove 'fetch' workaround 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. * fix(fetchtool): catch fetch exception on get resource * fix(fetchtool): update another fetch * fix(fetchtool, webhelper): handle asset requests with a 404 response 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 * test(fetchtool): test handling 404 response to get request Add a test that FetchTool.get resolves null when it encounters a 404 for the get request * fix(fetchtool): create Uint8Array on promise result instead of promise We were previously calling new Uint8Array on a promise instead of the result of the promise * fix(fetchworkertool.worker): update `get` to handle 404 response * test(fetchtool): update tests to handle promise reject and resolve * fix(fetchworkertool): don't create Uint8Array for null response * fix(worker): report worker errors as strings Error objects can't be sent with `postMessage`, so report errors using strings instead. * fix(worker): protect against null/undefined errors Co-authored-by: Karishma Chadha <[email protected]> * feat(circleci): add circleci * ci: use node orb and node 16 * test: fix and modernize tests * test: stop using deprecated methods * chore: upgrade package-lock.json to lockfileVersion 2 ```sh npm ci && npm i --package-lock-only && npm i --package-lock-only ``` * refactor: use cross-fetch instead of node-fetch 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. * chore: insert scratchFetch into FetchTool's pipeline * feat: allow adding metadata to scratchFetch * fix: apply metadata to FetchWorkerTool too * fix: don't use ?. in src/ because VM isn't ready for that * feat: allow removing request metadata * docs: clarify (and fix) jsdoc for is*Supported in FetchTool * fix: send Headers to fetch worker safely Without this, the Headers object gets corrupted by postMessage and results in CORS errors complaining about a header field called "map" * test: expose scratchFetch API better to support testing * refactor: store & merge headers using Headers object 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. * test: mock cross-fetch more completely Also emphasize that the `Headers` class used in `scratchFetch.js` comes from `cross-fetch` * refactor: turn mockFetch into a full module mock This should make it harder to make a mistake mocking `cross-fetch` again * test: add test for case sensitivity in metadata keys * fix: rename headers to fit HTTP convention * chore(deps): pin node orb to 5.1.0 * refactor: don't use !arraybuffer-loader! notation * test: wait for checkAsset in load-default-assets tests * test: convert all tests from node-tap to jest * test: mock cross-fetch for download-known-assets, too * test: fix transformer for backslashes 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. * feat: control scratchFetch with query param * test: add '?scratchMetadata=1' in metadata test * fix: tell browsers to use browser build * test: verify default asset sizes This checks for things like accidentally using the filename as the file content, etc. * chore: repo rename for npm publish * chore: package-lock update * build: github actions build * build: ci pin node version --------- Co-authored-by: Christopher Willis-Ford <[email protected]> Co-authored-by: Karishma Chadha <[email protected]> Co-authored-by: meyerhot95 <[email protected]> Co-authored-by: meyerhot95 <[email protected]> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
- Loading branch information
6 people
authored
Oct 30, 2023
1 parent
d12fc4a
commit 22bf32b