diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 619b25b8..b0f75f3b 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -37,12 +37,19 @@ jobs: GitHub head ref: ${{ github.head_ref }} EOF - - name: Setup & Test + - run: npm ci + + - name: Pre-build tests run: | - npm ci - npm run build + npm run clean npm run test + - name: Build + run: npm run build + + - name: Post-build tests + run: npm run test:build + - name: Semantic release (configured to run dry if branch is other than 'develop') env: NPM_TOKEN: ${{ secrets.NPM_TOKEN }} diff --git a/package-lock.json b/package-lock.json index 72b176d5..f3d75c90 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,7 +13,6 @@ "arraybuffer-loader": "^1.0.3", "base64-js": "^1.3.0", "buffer": "6.0.3", - "cross-fetch": "^4.1.0", "fastestsmallesttextencoderdecoder": "^1.0.7", "js-md5": "^0.7.3", "minilog": "^3.1.0" @@ -8222,15 +8221,6 @@ "node": "^14.15.0 || ^16.10.0 || >=18.0.0" } }, - "node_modules/cross-fetch": { - "version": "4.1.0", - "resolved": "https://registry.npmjs.org/cross-fetch/-/cross-fetch-4.1.0.tgz", - "integrity": "sha512-uKm5PU+MHTootlWEY+mZ4vvXoCn4fLQxT9dSc1sXVMSFkINTJVN8cAQROpwcKm8bJ/c7rgZVIBWzH5T78sNZZw==", - "license": "MIT", - "dependencies": { - "node-fetch": "^2.7.0" - } - }, "node_modules/cross-spawn": { "version": "7.0.6", "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.6.tgz", @@ -14451,6 +14441,7 @@ "version": "2.7.0", "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.7.0.tgz", "integrity": "sha512-c4FRfUm/dbcWZ7U+1Wq0AwCyFL+3nt2bEw05wfxSz+DWpWsitgmSgYmy2dQdWyKC1694ELPqMs/YzUSNozLt8A==", + "dev": true, "license": "MIT", "dependencies": { "whatwg-url": "^5.0.0" @@ -20180,6 +20171,7 @@ "version": "0.0.3", "resolved": "https://registry.npmjs.org/tr46/-/tr46-0.0.3.tgz", "integrity": "sha512-N3WMsuqV66lT30CrXNbEjx4GEwlow3v6rr4mCcv6prnfwhS01rkgyFdjPNBYd9br7LpXV1+Emh01fHnq2Gdgrw==", + "dev": true, "license": "MIT" }, "node_modules/traverse": { @@ -21132,6 +21124,7 @@ "version": "3.0.1", "resolved": "https://registry.npmjs.org/webidl-conversions/-/webidl-conversions-3.0.1.tgz", "integrity": "sha512-2JAn3z8AR6rjK8Sm8orRC0h/bcl/DqL7tRPdGZ4I1CjdF+EaMLmYxBHyXuKL849eucPFhvBoxMsflfOb8kxaeQ==", + "dev": true, "license": "BSD-2-Clause" }, "node_modules/webpack": { @@ -21302,6 +21295,7 @@ "version": "5.0.0", "resolved": "https://registry.npmjs.org/whatwg-url/-/whatwg-url-5.0.0.tgz", "integrity": "sha512-saE57nupxk6v3HY35+jzBwYa0rKSy0XR8JSxZPwgLr7ys0IBzhGviA1/TUGJLmSVqs8pb9AnvICXEuOHLprYTw==", + "dev": true, "license": "MIT", "dependencies": { "tr46": "~0.0.3", diff --git a/package.json b/package.json index 2d130298..a86d6bc1 100644 --- a/package.json +++ b/package.json @@ -13,12 +13,14 @@ "browser": "./dist/web/scratch-storage.js", "types": "./dist/types/index.d.ts", "scripts": { - "build": "rimraf dist && webpack --mode=production", + "build": "npm run clean && webpack --mode=production", + "clean": "rimraf dist", "commitmsg": "commitlint -e $GIT_PARAMS", "coverage": "tap ./test/{unit,integration}/*.js --coverage --coverage-report=lcov", "prepare": "husky", "semantic-release": "semantic-release", "test": "npm run test:lint && jest \"test[\\\\/](unit|integration)\"", + "test:build": "jest \"test[\\\\/]build\"", "test:clearCache": "jest --clearCache", "test:integration": "jest \"test[\\\\/]integration\"", "test:lint": "eslint", @@ -34,7 +36,6 @@ "arraybuffer-loader": "^1.0.3", "base64-js": "^1.3.0", "buffer": "6.0.3", - "cross-fetch": "^4.1.0", "fastestsmallesttextencoderdecoder": "^1.0.7", "js-md5": "^0.7.3", "minilog": "^3.1.0" diff --git a/src/FetchTool.ts b/src/FetchTool.ts index 91390496..61d097e7 100644 --- a/src/FetchTool.ts +++ b/src/FetchTool.ts @@ -1,3 +1,4 @@ +import {AssetQueueOptions} from './HostQueues'; import {scratchFetch} from './scratchFetch'; import {ScratchGetRequest, ScratchSendRequest, Tool} from './Tool'; @@ -24,7 +25,7 @@ export class FetchTool implements Tool { * @returns {Promise.} Resolve to Buffer of data from server. */ get ({url, ...options}: ScratchGetRequest): Promise { - return scratchFetch(url, Object.assign({method: 'GET'}, options)) + return scratchFetch(url, Object.assign({method: 'GET'}, options), {queueOptions: AssetQueueOptions}) .then((result: Response) => { if (result.ok) return result.arrayBuffer().then(b => new Uint8Array(b)); if (result.status === 404) return null; @@ -49,7 +50,7 @@ export class FetchTool implements Tool { send ({url, withCredentials = false, ...options}: ScratchSendRequest): Promise { return scratchFetch(url, Object.assign({ credentials: withCredentials ? 'include' : 'omit' - }, options)) + }, options), {queueOptions: AssetQueueOptions}) .then(response => { if (response.ok) return response.text(); return Promise.reject(response.status); diff --git a/src/FetchWorkerTool.worker.js b/src/FetchWorkerTool.worker.js deleted file mode 100644 index 06dc5e5b..00000000 --- a/src/FetchWorkerTool.worker.js +++ /dev/null @@ -1,67 +0,0 @@ -/* eslint-env worker */ - -const crossFetch = require('cross-fetch').default; - -let jobsActive = 0; -const complete = []; - -let intervalId = null; - -/** - * Register a step function. - * - * Step checks if there are completed jobs and if there are sends them to the - * parent. Then it checks the jobs count. If there are no further jobs, clear - * the step. - */ -const registerStep = function () { - intervalId = setInterval(() => { - if (complete.length) { - // Send our chunk of completed requests and instruct postMessage to - // transfer the buffers instead of copying them. - postMessage( - complete.slice(), - // Instruct postMessage that these buffers in the sent message - // should use their Transferable trait. After the postMessage - // call the "buffers" will still be in complete if you looked, - // but they will all be length 0 as the data they reference has - // been sent to the window. This lets us send a lot of data - // without the normal postMessage behaviour of making a copy of - // all of the data for the window. - complete.map(response => response.buffer).filter(Boolean) - ); - complete.length = 0; - } - if (jobsActive === 0) { - clearInterval(intervalId); - intervalId = null; - } - }, 1); -}; - -/** - * Receive a job from the parent and fetch the requested data. - * @param {object} message The message from the parent. - * @param {object} message.data A job id, url, and options descriptor to perform. - */ -const onMessage = ({data: job}) => { - if (jobsActive === 0 && !intervalId) { - registerStep(); - } - - jobsActive++; - - crossFetch(job.url, job.options) - .then(result => { - if (result.ok) return result.arrayBuffer(); - if (result.status === 404) return null; - return Promise.reject(result.status); - }) - .then(buffer => complete.push({id: job.id, buffer})) - .catch(error => complete.push({id: job.id, error: (error && error.message) || `Failed request: ${job.url}`})) - .then(() => jobsActive--); -}; - -// crossFetch means "fetch" is now always supported -postMessage({support: {fetch: true}}); -self.addEventListener('message', onMessage); diff --git a/src/FetchWorkerTool.worker.ts b/src/FetchWorkerTool.worker.ts new file mode 100644 index 00000000..9825c87f --- /dev/null +++ b/src/FetchWorkerTool.worker.ts @@ -0,0 +1,94 @@ +/* eslint-env worker */ +/* eslint-disable-next-line spaced-comment */ +/// + +// This worker won't share the same queue as the main thread, but throttling should be okay +// as long as we don't use FetchTool and FetchWorkerTool at the same time. +// TODO: Communicate metadata from the main thread to workers or move the worker boundary "into" `scratchFetch`. +// Make sure to benchmark any changes to avoid performance regressions, especially for large project loads. +import {AssetQueueOptions} from './HostQueues'; +import {scratchFetch} from './scratchFetch'; + +interface JobMessage { + id: string; + url: string; + options: RequestInit | undefined; +} + +interface CompletionMessage { + id: string; + buffer?: ArrayBuffer | null; + error?: string; +} + +let jobsActive = 0; +const complete: CompletionMessage[] = []; + +let intervalId: ReturnType | undefined = void 0; + +/** + * Register a step function. + * + * Step checks if there are completed jobs and if there are sends them to the + * parent. Then it checks the jobs count. If there are no further jobs, clear + * the step. + */ +const registerStep = function () { + intervalId = setInterval(() => { + if (complete.length) { + // Send our chunk of completed requests and instruct postMessage to + // transfer the buffers instead of copying them. + postMessage( + complete.slice(), + // Instruct postMessage that these buffers in the sent message + // should use their Transferable trait. After the postMessage + // call the "buffers" will still be in complete if you looked, + // but they will all be length 0 as the data they reference has + // been sent to the window. This lets us send a lot of data + // without the normal postMessage behaviour of making a copy of + // all of the data for the window. + complete.map(response => response.buffer).filter(Boolean) as Transferable[] + ); + complete.length = 0; + } + if (jobsActive === 0) { + clearInterval(intervalId); + intervalId = void 0; + } + }, 1); +}; + +/** + * Receive a job from the parent and fetch the requested data. + * @param message The message from the parent. + * @param message.data A job id, url, and options descriptor to perform. + */ +const onMessage = async ({data: job}: MessageEvent) => { + if (jobsActive === 0 && !intervalId) { + registerStep(); + } + + jobsActive++; + + try { + const response = await scratchFetch(job.url, job.options, {queueOptions: AssetQueueOptions}); + + const result: CompletionMessage = {id: job.id}; + if (response.ok) { + result.buffer = await response.arrayBuffer(); + } else if (response.status === 404) { + result.buffer = null; + } else { + throw response.status; + } + complete.push(result); + } catch (error) { + complete.push({id: job.id, error: ((error as Error)?.message) || `Failed request: ${job.url}`}); + } finally { + jobsActive--; + } +}; + +// "fetch" is supported in Node.js as of 16.15 and our target browsers as of ~2017 +postMessage({support: {fetch: true}}); +self.addEventListener('message', onMessage); diff --git a/src/HostQueues.ts b/src/HostQueues.ts new file mode 100644 index 00000000..7000c6db --- /dev/null +++ b/src/HostQueues.ts @@ -0,0 +1,33 @@ +import {QueueManager, type QueueOptions} from '@scratch/task-herder'; + +/** + * @summary A set of generous limits, for things like downloading assets from CDN. + * @description + * In practice, these limits seem to lead to slightly better performance than no limits at all, mostly due to the + * concurrency limit. For example, on my development computer & my relatively fast residential connection, a + * concurrency limit of 4 loads a particular test project in 21 seconds, as opposed to 25 seconds when I bypass the + * queue and call `fetch` directly. In that test, my setup downloads about 50 assets per second, so this set of options + * only affects concurrency and doesn't actually throttle the downloads. Limiting concurrency also fixes the issue + * where very large projects (thousands of assets) can lead to browser failures like `net::ERR_INSUFFICIENT_RESOURCES`. + * The exact concurrency limit doesn't seem to matter much since the browser limits parallel connections itself. It + * just needs to be high enough to avoid bubbles in the download pipeline and low enough to avoid resource exhaustion. + * @see {@link https://github.com/scratchfoundation/scratch-gui/issues/7111} + */ +export const AssetQueueOptions: QueueOptions = { + burstLimit: 64, + sustainRate: 64, + // WARNING: asset download concurrency >=5 can lead to corrupted buffers on Chrome (December 2025, Chrome 142.0) + // when using Scratch's bitmap load pipeline. Marking the canvas context as `{willReadFrequently: true}` seems to + // eliminate that issue, so maybe the problem is related to hardware acceleration. + concurrency: 64 +}; + +/** + * Central registry of per-host queues. + * Uses strict limits by default. Override these strict limits as needed for specific hosts. + */ +export const hostQueueManager = new QueueManager({ + burstLimit: 5, + sustainRate: 1, + concurrency: 1 +}); diff --git a/src/ScratchStorage.ts b/src/ScratchStorage.ts index b04e29a5..4783185d 100644 --- a/src/ScratchStorage.ts +++ b/src/ScratchStorage.ts @@ -6,7 +6,7 @@ import WebHelper, {UrlFunction} from './WebHelper'; import _Asset, {AssetData, AssetId} from './Asset'; import {AssetType as _AssetType, AssetType} from './AssetType'; import {DataFormat as _DataFormat, DataFormat} from './DataFormat'; -import _scratchFetch from './scratchFetch'; +import * as _scratchFetch from './scratchFetch'; import Helper from './Helper'; interface HelperWithPriority { diff --git a/src/scratchFetch.js b/src/scratchFetch.ts similarity index 55% rename from src/scratchFetch.js rename to src/scratchFetch.ts index c029dd4d..cd766dc1 100644 --- a/src/scratchFetch.js +++ b/src/scratchFetch.ts @@ -1,29 +1,44 @@ -const crossFetch = require('cross-fetch'); +import {type QueueOptions} from '../../scratch-editor/packages/task-herder/dist/TaskQueue'; +import {hostQueueManager} from './HostQueues'; + +export const Headers = globalThis.Headers; /** - * Metadata header names - * @enum {string} The enum value is the name of the associated header. - * @readonly + * Metadata header names. + * The enum value is the name of the associated header. */ -const RequestMetadata = { +export enum RequestMetadata { /** The ID of the project associated with this request */ - ProjectId: 'X-Project-ID', + ProjectId = 'X-Project-ID', /** The ID of the project run associated with this request */ - RunId: 'X-Run-ID' + RunId = 'X-Run-ID' +} + +export type ScratchFetchOptions = { + /** + * The name of the queue to use for this request. + * If absent, the hostname of the requested URL will be used as the queue name. + * This is a Scratch-specific extension to the standard RequestInit type. + */ + queueName?: string; + + /** + * The options to use when creating the queue for this request. + * Ignored if a queue with the specified name already exists. + */ + queueOptions?: QueueOptions; }; /** - * Metadata headers for requests - * @type {Headers} + * Metadata headers for requests. */ -const metadata = new crossFetch.Headers(); +const metadata = new Headers(); /** * Check if there is any metadata to apply. * @returns {boolean} true if `metadata` has contents, or false if it is empty. */ -const hasMetadata = () => { - /* global self */ +export const hasMetadata = (): boolean => { const searchParams = ( typeof self !== 'undefined' && self && @@ -51,16 +66,16 @@ const hasMetadata = () => { * @param {RequestInit} [options] The initial request options. May be null or undefined. * @returns {RequestInit|undefined} the provided options parameter without modification, or a new options object. */ -const applyMetadata = options => { +export const applyMetadata = (options?: globalThis.RequestInit): globalThis.RequestInit | undefined => { if (hasMetadata()) { const augmentedOptions = Object.assign({}, options); - augmentedOptions.headers = new crossFetch.Headers(metadata); + augmentedOptions.headers = new Headers(metadata); if (options && options.headers) { // the Fetch spec says options.headers could be: // "A Headers object, an object literal, or an array of two-item arrays to set request's headers." // turn it into a Headers object to be sure of how to interact with it - const overrideHeaders = options.headers instanceof crossFetch.Headers ? - options.headers : new crossFetch.Headers(options.headers); + const overrideHeaders = options.headers instanceof Headers ? + options.headers : new Headers(options.headers); for (const [name, value] of overrideHeaders.entries()) { augmentedOptions.headers.set(name, value); } @@ -74,13 +89,27 @@ const applyMetadata = options => { * Make a network request. * This is a wrapper for the global fetch method, adding some Scratch-specific functionality. * @param {RequestInfo|URL} resource The resource to fetch. - * @param {RequestInit} options Optional object containing custom settings for this request. + * @param {RequestInit} [requestOptions] Optional object containing custom settings for this request. + * @param {ScratchFetchOptions} [scratchOptions] Optional Scratch-specific settings for this request. * @see {@link https://developer.mozilla.org/docs/Web/API/fetch} for more about the fetch API. * @returns {Promise} A promise for the response to the request. */ -const scratchFetch = (resource, options) => { - const augmentedOptions = applyMetadata(options); - return crossFetch(resource, augmentedOptions); +export const scratchFetch = ( + resource: RequestInfo | URL, + requestOptions?: globalThis.RequestInit, + scratchOptions?: ScratchFetchOptions +): Promise => { + requestOptions = applyMetadata(requestOptions); + + let queueName = scratchOptions?.queueName; + if (!queueName) { + // Normalize resource to a Request object. The `fetch` call will do this anyway, so it's not much extra work, + // but it guarantees availability of the URL for queue naming. + resource = new Request(resource, requestOptions); + queueName = new URL(resource.url).hostname; + } + const queue = hostQueueManager.getOrCreate(queueName, scratchOptions?.queueOptions); + return queue.do(() => fetch(resource, requestOptions)); }; /** @@ -90,7 +119,7 @@ const scratchFetch = (resource, options) => { * @param {RequestMetadata} name The name of the metadata item to set. * @param {any} value The value to set (will be converted to a string). */ -const setMetadata = (name, value) => { +export const setMetadata = (name: RequestMetadata, value: any): void => { metadata.set(name, value); }; @@ -98,7 +127,7 @@ const setMetadata = (name, value) => { * Remove a named request metadata item. * @param {RequestMetadata} name The name of the metadata item to remove. */ -const unsetMetadata = name => { +export const unsetMetadata = (name: RequestMetadata): void => { metadata.delete(name); }; @@ -106,16 +135,6 @@ const unsetMetadata = name => { * Retrieve a named request metadata item. * Only for use in tests. At the time of writing, used in scratch-vm tests. * @param {RequestMetadata} name The name of the metadata item to retrieve. - * @returns {any} value The value of the metadata item, or `undefined` if it was not found. + * @returns {string|null} The value of the metadata item, or `null` if it was not found. */ -const getMetadata = name => metadata.get(name); - -module.exports = { - Headers: crossFetch.Headers, - RequestMetadata, - applyMetadata, - scratchFetch, - setMetadata, - unsetMetadata, - getMetadata -}; +export const getMetadata = (name: RequestMetadata): string | null => metadata.get(name); diff --git a/src/types.d.ts b/src/types.d.ts index e3a95c40..e88f1bb4 100644 --- a/src/types.d.ts +++ b/src/types.d.ts @@ -1,3 +1,4 @@ -declare module '*.png?arrayBuffer'; -declare module '*.wav?arrayBuffer'; -declare module '*.svg?arrayBuffer'; +declare module '*?arrayBuffer' { + const value: ArrayBuffer; + export default value; +} diff --git a/test/build/api.test.js b/test/build/api.test.js new file mode 100644 index 00000000..aa8fb978 --- /dev/null +++ b/test/build/api.test.js @@ -0,0 +1,60 @@ +/** + * @file Tests the build output to verify the general parts of the public API. + */ + +const ScratchStorageModule = require('../../dist/node/scratch-storage.js'); + +/** @type {ScratchStorageModule.ScratchStorage} */ +let storage; + +beforeEach(() => { + const {ScratchStorage} = ScratchStorageModule; + storage = new ScratchStorage(); +}); + +test('constructor', () => { + const {ScratchStorage} = ScratchStorageModule; + expect(storage).toBeInstanceOf(ScratchStorage); +}); + +test('DataFormat', () => { + const {DataFormat} = ScratchStorageModule; + expect(DataFormat).toBeDefined(); + expect(DataFormat.JPG).toBe('jpg'); + expect(DataFormat.JSON).toBe('json'); + expect(DataFormat.MP3).toBe('mp3'); + expect(DataFormat.PNG).toBe('png'); + expect(DataFormat.SB2).toBe('sb2'); + expect(DataFormat.SB3).toBe('sb3'); + expect(DataFormat.SVG).toBe('svg'); + expect(DataFormat.WAV).toBe('wav'); +}); + +test('AssetType', () => { + const {AssetType, DataFormat} = ScratchStorageModule; + expect(AssetType).toBeDefined(); + expect(AssetType.ImageBitmap.contentType).toBe('image/png'); + expect(AssetType.ImageVector.contentType).toBe('image/svg+xml'); + expect(AssetType.Project.runtimeFormat).toBe(DataFormat.JSON); + expect(AssetType.Sound.immutable).toBe(true); + expect(AssetType.Sprite.name).toBe('Sprite'); +}); + +test('Asset', () => { + const {Asset, AssetType} = ScratchStorageModule; + expect(Asset).toBeDefined(); + const asset = new Asset( + AssetType.ImageVector, + 'some-hash' + ); + expect(asset).toBeInstanceOf(Asset); + expect(asset.dataFormat).toBe('svg'); + expect(asset.assetId).toBe('some-hash'); +}); + +test('Helper', () => { + const {Helper} = ScratchStorageModule; + expect(Helper).toBeDefined(); + const helper = new Helper(); + expect(helper).toBeInstanceOf(Helper); +}); diff --git a/test/build/scratchFetch.test.js b/test/build/scratchFetch.test.js new file mode 100644 index 00000000..d0cc6d44 --- /dev/null +++ b/test/build/scratchFetch.test.js @@ -0,0 +1,40 @@ +/** + * @file Tests the build output to verify the scratchFetch portion of the public API. + */ + +const ScratchStorageModule = require('../../dist/node/scratch-storage.js'); + +/** @type {ScratchStorageModule.ScratchStorage} */ +let storage; + +beforeEach(() => { + const {ScratchStorage} = ScratchStorageModule; + storage = new ScratchStorage(); +}); + +test('scratchFetch accessor', () => { + expect(storage.scratchFetch).toBeDefined(); +}); + +test('Headers', () => { + expect(storage.scratchFetch).toBeDefined(); + const headers = new storage.scratchFetch.Headers(); + expect(headers).toBeInstanceOf(storage.scratchFetch.Headers); +}); + +test('RequestMetadata enum', () => { + expect(storage.scratchFetch.RequestMetadata).toBeDefined(); + expect(typeof storage.scratchFetch.RequestMetadata.ProjectId).toBe('string'); + expect(typeof storage.scratchFetch.RequestMetadata.RunId).toBe('string'); +}); + +test('scratchFetch function', () => { + expect(typeof storage.scratchFetch.scratchFetch).toBe('function'); +}); + +test('metadata functions', () => { + expect(typeof storage.scratchFetch.applyMetadata).toBe('function'); + expect(typeof storage.scratchFetch.setMetadata).toBe('function'); + expect(typeof storage.scratchFetch.unsetMetadata).toBe('function'); + expect(typeof storage.scratchFetch.getMetadata).toBe('function'); +}); diff --git a/test/fixtures/known-assets.js b/test/fixtures/known-assets.js index 9a5794ac..da65940a 100644 --- a/test/fixtures/known-assets.js +++ b/test/fixtures/known-assets.js @@ -3,6 +3,16 @@ const path = require('path'); const md5 = require('js-md5'); +/** + * @typedef {object} KnownAsset + * @property {Buffer} content - The content of the asset. + * @property {string} hash - The MD5 hash of the asset content. + */ + +/** + * @typedef {{[id: string]: KnownAsset}} KnownAssetCollection + */ + const projects = [ '117504922' ]; @@ -15,6 +25,11 @@ const assets = [ 'fe5e3566965f9de793beeffce377d054.jpg' ]; +/** + * Load a file from disk, then return its content and hash. + * @param {string} filename - The file to load + * @returns {KnownAsset} The loaded asset + */ const loadSomething = filename => { const fullPath = path.join(__dirname, 'assets', filename); const content = fs.readFileSync(fullPath); @@ -25,6 +40,11 @@ const loadSomething = filename => { }; }; +/** + * Load a project from disk, ensure it's valid JSON, then return its content and hash. + * @param {string} id - The project ID + * @returns {KnownAsset} The loaded project asset + */ const loadProject = id => { const filename = `${id}.json`; const result = loadSomething(filename); @@ -35,6 +55,11 @@ const loadProject = id => { return result; }; +/** + * Load an asset from disk, ensuring its hash matches its filename. + * @param {string} filename - The file to load + * @returns {KnownAsset} The loaded asset + */ const loadAsset = filename => { const result = loadSomething(filename); @@ -46,15 +71,18 @@ const loadAsset = filename => { return result; }; +/** + * @type {KnownAssetCollection} + */ const knownAssets = Object.assign({}, projects.reduce((bag, id) => { bag[id] = loadProject(id); return bag; - }, {}), + }, /** @type {KnownAssetCollection} */ ({})), assets.reduce((bag, filename) => { bag[filename] = loadAsset(filename); return bag; - }, {}) + }, /** @type {KnownAssetCollection} */ ({})) ); module.exports = knownAssets; diff --git a/test/__mocks__/cross-fetch.js b/test/fixtures/mockFetch.js similarity index 80% rename from test/__mocks__/cross-fetch.js rename to test/fixtures/mockFetch.js index 6a9be08f..a4419247 100644 --- a/test/__mocks__/cross-fetch.js +++ b/test/fixtures/mockFetch.js @@ -1,8 +1,15 @@ +/** + * Mock implementation of the fetch function for testing. + * Since `fetch` is a global, Jest will not automatically mock it from `__mocks__`. + * + * // In your test setup file or at the top of your test files: + * global.fetch = require('../mocks/fetch').default; + */ + const TextEncoder = require('util').TextEncoder; -const crossFetch = jest.requireActual('cross-fetch'); const knownAssets = require('../fixtures/known-assets.js'); -const Headers = crossFetch.Headers; +const Headers = global.Headers; const successText = 'successful response'; /** @@ -30,7 +37,7 @@ const successText = 'successful response'; * @returns {Promise} A promise for a Response-like object. Does not fully implement Response. */ const mockFetch = (resource, options) => { - /** @type MockFetchResponse */ + /** @type {MockFetchResponse} */ const results = { ok: false, status: 0 @@ -39,14 +46,15 @@ const mockFetch = (resource, options) => { options.mockFetchTestData.headers = new Headers(options.headers); options.mockFetchTestData.headersCount = Array.from(options.mockFetchTestData.headers).length; } - - const assetInfo = knownAssets[resource]; + const request = new Request(resource, options); + const path = new URL(request.url).pathname.slice(1); // remove leading '/' + const assetInfo = knownAssets[path]; if (assetInfo) { results.ok = true; results.status = 200; results.arrayBuffer = () => Promise.resolve(assetInfo.content); } else { - switch (resource) { + switch (path) { case '200': results.ok = true; results.status = 200; @@ -68,14 +76,7 @@ const mockFetch = (resource, options) => { return Promise.resolve(results); }; -// Mimic the cross-fetch module, but replace its `fetch` with `mockFetch` and add a few extras - -module.exports = exports = mockFetch; -exports.fetch = mockFetch; -exports.Headers = crossFetch.Headers; -exports.Request = crossFetch.Request; -exports.Response = crossFetch.Response; -exports.successText = successText; - -// Needed for TypeScript consumers without esModuleInterop. -exports.default = mockFetch; +module.exports = { + fetch: mockFetch, + successText +}; diff --git a/test/integration/download-known-assets.test.js b/test/integration/download-known-assets.test.js index d33c4d1e..00687eb9 100644 --- a/test/integration/download-known-assets.test.js +++ b/test/integration/download-known-assets.test.js @@ -1,7 +1,10 @@ const md5 = require('js-md5'); +const mockFetch = require('../fixtures/mockFetch.js'); const ScratchStorage = require('../../src/index').ScratchStorage; +jest.spyOn(global, 'fetch').mockImplementation(mockFetch.fetch); + test('constructor', () => { const storage = new ScratchStorage(); expect(storage).toBeInstanceOf(ScratchStorage); @@ -68,15 +71,15 @@ const getTestAssets = storage => [ ]; const addWebStores = storage => { - // these `asset => ...` callbacks generate values specifically for the cross-fetch mock + // these `asset => ...` callbacks generate values specifically for the fetch mock // in the real world they would generate proper URIs storage.addWebStore( [storage.AssetType.Project], - asset => asset.assetId, + asset => `http://example.com/${asset.assetId}`, null, null); storage.addWebStore( [storage.AssetType.ImageVector, storage.AssetType.ImageBitmap, storage.AssetType.Sound], - asset => `${asset.assetId}.${asset.dataFormat}`, + asset => `http://example.com/${asset.assetId}.${asset.dataFormat}`, null, null ); }; diff --git a/test/unit/fetch-tool.test.js b/test/unit/fetch-tool.test.js index 0fe548a9..3964035f 100644 --- a/test/unit/fetch-tool.test.js +++ b/test/unit/fetch-tool.test.js @@ -1,13 +1,14 @@ const TextDecoder = require('util').TextDecoder; -jest.mock('cross-fetch'); -const mockFetch = require('cross-fetch'); +const mockFetch = require('../fixtures/mockFetch.js'); +jest.spyOn(global, 'fetch').mockImplementation(mockFetch.fetch); + const {FetchTool} = require('../../src/FetchTool'); test('send success returns response.text()', async () => { const tool = new FetchTool(); - const result = await tool.send({url: '200'}); + const result = await tool.send({url: 'http://example.com/200'}); expect(result).toBe(mockFetch.successText); }); @@ -17,7 +18,7 @@ test('send failure returns response.status', async () => { const catcher = jest.fn(); try { - await tool.send({url: '500'}); + await tool.send({url: 'http://example.com/500'}); } catch (e) { catcher(e); } @@ -31,14 +32,14 @@ test('get success returns Uint8Array.body(response.arrayBuffer())', async () => const tool = new FetchTool(); - const result = await tool.get({url: '200'}); + const result = await tool.get({url: 'http://example.com/200'}); expect(decoder.decode(result)).toBe(mockFetch.successText); }); test('get with 404 response returns null data', async () => { const tool = new FetchTool(); - const result = await tool.get({url: '404'}); + const result = await tool.get({url: 'http://example.com/404'}); expect(result).toBeNull(); }); @@ -47,7 +48,7 @@ test('get failure returns response.status', async () => { const catcher = jest.fn(); try { - await tool.get({url: '500'}); + await tool.get({url: 'http://example.com/500'}); } catch (e) { catcher(e); } diff --git a/test/unit/metadata.test.js b/test/unit/metadata.test.js index 5d98d77d..a31bdc0f 100644 --- a/test/unit/metadata.test.js +++ b/test/unit/metadata.test.js @@ -1,4 +1,5 @@ -jest.mock('cross-fetch'); +const mockFetch = require('../fixtures/mockFetch.js'); +jest.spyOn(global, 'fetch').mockImplementation(mockFetch.fetch); beforeEach(() => { // reset the metadata container to ensure the tests don't interfere with each other @@ -17,7 +18,7 @@ test('get without metadata', async () => { const tool = new FetchTool(); const mockFetchTestData = {}; - const result = await tool.get({url: '200', mockFetchTestData}); + const result = await tool.get({url: 'http://example.com/200', mockFetchTestData}); expect(result).toBeInstanceOf(Uint8Array); expect(mockFetchTestData.headers).toBeTruthy(); @@ -35,7 +36,7 @@ test('get with metadata', async () => { setMetadata(RequestMetadata.RunId, 5678); const mockFetchTestData = {}; - const result = await tool.get({url: '200', mockFetchTestData}); + const result = await tool.get({url: 'http://example.com/200', mockFetchTestData}); expect(result).toBeInstanceOf(Uint8Array); expect(mockFetchTestData.headers).toBeTruthy(); @@ -49,7 +50,7 @@ test('send without metadata', async () => { const tool = new FetchTool(); const mockFetchTestData = {}; - const result = await tool.send({url: '200', mockFetchTestData}); + const result = await tool.send({url: 'http://example.com/200', mockFetchTestData}); expect(typeof result).toBe('string'); expect(mockFetchTestData.headers).toBeTruthy(); @@ -67,7 +68,7 @@ test('send with metadata', async () => { setMetadata(RequestMetadata.RunId, 8765); const mockFetchTestData = {}; - const result = await tool.send({url: '200', mockFetchTestData}); + const result = await tool.send({url: 'http://example.com/200', mockFetchTestData}); expect(typeof result).toBe('string'); expect(mockFetchTestData.headers).toBeTruthy(); @@ -89,7 +90,7 @@ test('selectively delete metadata', async () => { const mockFetchTestData = {}; - const result1 = await tool.send({url: '200', mockFetchTestData}); + const result1 = await tool.send({url: 'http://example.com/200', mockFetchTestData}); expect(typeof result1).toBe('string'); expect(mockFetchTestData.headers).toBeTruthy(); @@ -100,7 +101,7 @@ test('selectively delete metadata', async () => { // remove the Project ID from metadata unsetMetadata(RequestMetadata.ProjectId); - const result2 = await tool.send({url: '200', mockFetchTestData}); + const result2 = await tool.send({url: 'http://example.com/200', mockFetchTestData}); expect(typeof result2).toBe('string'); expect(mockFetchTestData.headers).toBeTruthy(); @@ -120,7 +121,7 @@ test('metadata has case-insensitive keys', async () => { const tool = new FetchTool(); const mockFetchTestData = {}; - await tool.get({url: '200', mockFetchTestData}); + await tool.get({url: 'http://example.com/200', mockFetchTestData}); expect(mockFetchTestData.headers).toBeTruthy(); expect(mockFetchTestData.headersCount).toBe(1); diff --git a/tsconfig.json b/tsconfig.json index 900aee36..7ed522dc 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -9,7 +9,6 @@ /* Modules */ "module": "Preserve", - "types": ["./src/types.d.ts"], /* Emit */ "declaration": true, diff --git a/tsconfig.test.json b/tsconfig.test.json index 79a3c8f8..bce5bb17 100644 --- a/tsconfig.test.json +++ b/tsconfig.test.json @@ -5,6 +5,7 @@ /* Visit https://aka.ms/tsconfig to read more about this file */ /* Modules */ + "isolatedModules": true, // for ts-jest "module": "CommonJS", "types": ["jest", "./src/types.d.ts"], }