From 920d99e132f011f94ae820fef2ac70e15f644fe7 Mon Sep 17 00:00:00 2001 From: Lucas Correia Date: Wed, 21 Jul 2021 13:16:08 -0300 Subject: [PATCH] DevTools: Parse named source AST in a worker (#21902) Resolves #21855 Ended up using workerize in order to setup the worker once it allows easy imports (for babel's parse function) and exports. --- .../firefox/manifest.json | 2 +- .../react-devtools-extensions/package.json | 5 +- .../src/__tests__/parseHookNames-test.js | 131 +++++++++++++----- .../src/parseHookNames/index.js | 33 +++++ .../{ => parseHookNames}/parseHookNames.js | 17 ++- .../parseHookNames/parseHookNames.worker.js | 4 + .../webpack.config.js | 5 + .../src/hookNamesCache.js | 2 +- yarn.lock | 7 + 9 files changed, 161 insertions(+), 45 deletions(-) create mode 100644 packages/react-devtools-extensions/src/parseHookNames/index.js rename packages/react-devtools-extensions/src/{ => parseHookNames}/parseHookNames.js (98%) create mode 100644 packages/react-devtools-extensions/src/parseHookNames/parseHookNames.worker.js diff --git a/packages/react-devtools-extensions/firefox/manifest.json b/packages/react-devtools-extensions/firefox/manifest.json index a97422cedf86d..44327c8566f74 100644 --- a/packages/react-devtools-extensions/firefox/manifest.json +++ b/packages/react-devtools-extensions/firefox/manifest.json @@ -32,7 +32,7 @@ "devtools_page": "main.html", - "content_security_policy": "script-src 'self' 'unsafe-eval'; object-src 'self'", + "content_security_policy": "script-src 'self' 'unsafe-eval' blob:; object-src 'self'", "web_accessible_resources": [ "main.html", "panel.html", diff --git a/packages/react-devtools-extensions/package.json b/packages/react-devtools-extensions/package.json index da473f7613a68..4cb68ea8973a2 100644 --- a/packages/react-devtools-extensions/package.json +++ b/packages/react-devtools-extensions/package.json @@ -19,7 +19,6 @@ "update-mock-source-maps": "node ./src/__tests__/updateMockSourceMaps.js" }, "devDependencies": { - "acorn-jsx": "^5.2.0", "@babel/core": "^7.11.1", "@babel/plugin-proposal-class-properties": "^7.10.4", "@babel/plugin-transform-flow-strip-types": "^7.10.4", @@ -28,6 +27,7 @@ "@babel/preset-env": "^7.11.0", "@babel/preset-flow": "^7.10.4", "@babel/preset-react": "^7.10.4", + "acorn-jsx": "^5.2.0", "archiver": "^3.0.0", "babel-core": "^7.0.0-bridge", "babel-eslint": "^9.0.0", @@ -55,7 +55,8 @@ "web-ext": "^3.0.0", "webpack": "^4.43.0", "webpack-cli": "^3.3.11", - "webpack-dev-server": "^3.10.3" + "webpack-dev-server": "^3.10.3", + "workerize-loader": "^1.3.0" }, "dependencies": { "web-ext": "^4" diff --git a/packages/react-devtools-extensions/src/__tests__/parseHookNames-test.js b/packages/react-devtools-extensions/src/__tests__/parseHookNames-test.js index 0b10ffee2bee4..67e57871e4a13 100644 --- a/packages/react-devtools-extensions/src/__tests__/parseHookNames-test.js +++ b/packages/react-devtools-extensions/src/__tests__/parseHookNames-test.js @@ -11,6 +11,37 @@ // This is done to control if and how the code is transformed at runtime. // Do not declare test components within this test file as it is very fragile. +function expectHookNamesToEqual(map, expectedNamesArray) { + // Slightly hacky since it relies on the iterable order of values() + expect(Array.from(map.values())).toEqual(expectedNamesArray); +} + +function requireText(path, encoding) { + const {existsSync, readFileSync} = require('fs'); + if (existsSync(path)) { + return Promise.resolve(readFileSync(path, encoding)); + } else { + return Promise.reject(`File not found "${path}"`); + } +} + +const chromeGlobal = { + extension: { + getURL: jest.fn((...args) => { + const {join} = require('path'); + return join( + __dirname, + '..', + '..', + 'node_modules', + 'source-map', + 'lib', + 'mappings.wasm', + ); + }), + }, +}; + describe('parseHookNames', () => { let fetchMock; let inspectHooks; @@ -26,6 +57,9 @@ describe('parseHookNames', () => { fetchMock = require('jest-fetch-mock'); fetchMock.enableMocks(); + // Mock out portion of browser API used by parseHookNames to initialize "source-map". + global.chrome = chromeGlobal; + inspectHooks = require('react-debug-tools/src/ReactDebugHooks') .inspectHooks; parseHookNames = require('../parseHookNames').parseHookNames; @@ -45,44 +79,12 @@ describe('parseHookNames', () => { fetchMock.mockIf(/.+$/, request => { return requireText(request.url, 'utf8'); }); - - // Mock out portion of browser API used by parseHookNames to initialize "source-map". - global.chrome = { - extension: { - getURL: jest.fn((...args) => { - const {join} = require('path'); - return join( - __dirname, - '..', - '..', - 'node_modules', - 'source-map', - 'lib', - 'mappings.wasm', - ); - }), - }, - }; }); afterEach(() => { fetch.resetMocks(); }); - function expectHookNamesToEqual(map, expectedNamesArray) { - // Slightly hacky since it relies on the iterable order of values() - expect(Array.from(map.values())).toEqual(expectedNamesArray); - } - - function requireText(path, encoding) { - const {existsSync, readFileSync} = require('fs'); - if (existsSync(path)) { - return Promise.resolve(readFileSync(path, encoding)); - } else { - return Promise.reject(`File not found "${path}"`); - } - } - async function getHookNamesForComponent(Component, props = {}) { const hooksTree = inspectHooks(Component, props, undefined, true); const hookNames = await parseHookNames(hooksTree); @@ -344,3 +346,68 @@ describe('parseHookNames', () => { }); }); }); + +describe('parseHookNames worker', () => { + let inspectHooks; + let parseHookNames; + let originalParseHookNamesMock; + let workerizedParseHookNamesMock; + + beforeEach(() => { + window.Worker = undefined; + + originalParseHookNamesMock = jest.fn(); + workerizedParseHookNamesMock = jest.fn(); + + jest.mock('../parseHookNames/parseHookNames.js', () => { + return { + __esModule: true, + parseHookNames: originalParseHookNamesMock, + }; + }); + + jest.mock('../parseHookNames/parseHookNames.worker.js', () => { + return { + __esModule: true, + default: () => ({ + parseHookNames: workerizedParseHookNamesMock, + }), + }; + }); + + // Mock out portion of browser API used by parseHookNames to initialize "source-map". + global.chrome = chromeGlobal; + + inspectHooks = require('react-debug-tools/src/ReactDebugHooks') + .inspectHooks; + parseHookNames = require('../parseHookNames').parseHookNames; + }); + + async function getHookNamesForComponent(Component, props = {}) { + const hooksTree = inspectHooks(Component, props, undefined, true); + const hookNames = await parseHookNames(hooksTree); + return hookNames; + } + + it('should use worker when available', async () => { + const Component = require('./__source__/__untransformed__/ComponentWithUseState') + .Component; + + window.Worker = true; + // resets module so mocked worker instance can be updated + jest.resetModules(); + parseHookNames = require('../parseHookNames').parseHookNames; + + await getHookNamesForComponent(Component); + expect(workerizedParseHookNamesMock).toHaveBeenCalledTimes(1); + }); + + it('should use main thread when worker is not available', async () => { + const Component = require('./__source__/__untransformed__/ComponentWithUseState') + .Component; + + await getHookNamesForComponent(Component); + expect(workerizedParseHookNamesMock).toHaveBeenCalledTimes(0); + expect(originalParseHookNamesMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/react-devtools-extensions/src/parseHookNames/index.js b/packages/react-devtools-extensions/src/parseHookNames/index.js new file mode 100644 index 0000000000000..da864aa27085e --- /dev/null +++ b/packages/react-devtools-extensions/src/parseHookNames/index.js @@ -0,0 +1,33 @@ +/* global chrome */ + +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +// This file uses workerize to load ./parseHookNames.worker as a webworker +// and instanciates it, exposing flow typed functions that can be used +// on other files. + +import * as parseHookNamesModule from './parseHookNames'; +import WorkerizedParseHookNames from './parseHookNames.worker'; + +type ParseHookNamesModule = typeof parseHookNamesModule; + +// $FlowFixMe +const wasmMappingsURL = chrome.extension.getURL('mappings.wasm'); + +const workerizedParseHookNames: ParseHookNamesModule = window.Worker + ? WorkerizedParseHookNames() + : parseHookNamesModule; + +type ParseHookNames = $PropertyType; + +export const parseHookNames: ParseHookNames = hooksTree => + workerizedParseHookNames.parseHookNames(hooksTree, wasmMappingsURL); + +export const purgeCachedMetadata = workerizedParseHookNames.purgeCachedMetadata; diff --git a/packages/react-devtools-extensions/src/parseHookNames.js b/packages/react-devtools-extensions/src/parseHookNames/parseHookNames.js similarity index 98% rename from packages/react-devtools-extensions/src/parseHookNames.js rename to packages/react-devtools-extensions/src/parseHookNames/parseHookNames.js index 73745c6be3a1f..f68c37135b8b7 100644 --- a/packages/react-devtools-extensions/src/parseHookNames.js +++ b/packages/react-devtools-extensions/src/parseHookNames/parseHookNames.js @@ -1,5 +1,3 @@ -/* global chrome */ - /** * Copyright (c) Facebook, Inc. and its affiliates. * @@ -12,8 +10,8 @@ import {parse} from '@babel/parser'; import LRU from 'lru-cache'; import {SourceMapConsumer} from 'source-map'; -import {getHookName} from './astUtils'; -import {areSourceMapsAppliedToErrors} from './ErrorTester'; +import {getHookName} from '../astUtils'; +import {areSourceMapsAppliedToErrors} from '../ErrorTester'; import {__DEBUG__} from 'react-devtools-shared/src/constants'; import {getHookSourceLocationKey} from 'react-devtools-shared/src/hookNamesCache'; @@ -24,7 +22,7 @@ import type { } from 'react-debug-tools/src/ReactDebugHooks'; import type {HookNames, LRUCache} from 'react-devtools-shared/src/types'; import type {Thenable} from 'shared/ReactTypes'; -import type {SourceConsumer} from './astUtils'; +import type {SourceConsumer} from '../astUtils'; const SOURCE_MAP_REGEX = / ?sourceMappingURL=([^\s'"]+)/gm; const MAX_SOURCE_LENGTH = 100_000_000; @@ -103,6 +101,7 @@ const originalURLToMetadataCache: LRUCache< export async function parseHookNames( hooksTree: HooksTree, + wasmMappingsURL: string, ): Thenable { const hooksList: Array = []; flattenHooksList(hooksTree, hooksList); @@ -160,7 +159,9 @@ export async function parseHookNames( } return loadSourceFiles(locationKeyToHookSourceData) - .then(() => extractAndLoadSourceMaps(locationKeyToHookSourceData)) + .then(() => + extractAndLoadSourceMaps(locationKeyToHookSourceData, wasmMappingsURL), + ) .then(() => parseSourceAST(locationKeyToHookSourceData)) .then(() => updateLruCache(locationKeyToHookSourceData)) .then(() => findHookNames(hooksList, locationKeyToHookSourceData)); @@ -182,6 +183,7 @@ function decodeBase64String(encoded: string): Object { function extractAndLoadSourceMaps( locationKeyToHookSourceData: Map, + wasmMappingsURL: string, ): Promise<*> { // SourceMapConsumer.initialize() does nothing when running in Node (aka Jest) // because the wasm file is automatically read from the file system @@ -193,9 +195,6 @@ function extractAndLoadSourceMaps( ); } - // $FlowFixMe - const wasmMappingsURL = chrome.extension.getURL('mappings.wasm'); - SourceMapConsumer.initialize({'lib/mappings.wasm': wasmMappingsURL}); } diff --git a/packages/react-devtools-extensions/src/parseHookNames/parseHookNames.worker.js b/packages/react-devtools-extensions/src/parseHookNames/parseHookNames.worker.js new file mode 100644 index 0000000000000..7843527e4d963 --- /dev/null +++ b/packages/react-devtools-extensions/src/parseHookNames/parseHookNames.worker.js @@ -0,0 +1,4 @@ +import * as parseHookNamesModule from './parseHookNames'; + +export const parseHookNames = parseHookNamesModule.parseHookNames; +export const purgeCachedMetadata = parseHookNamesModule.purgeCachedMetadata; diff --git a/packages/react-devtools-extensions/webpack.config.js b/packages/react-devtools-extensions/webpack.config.js index 94a12ebdd1118..3b89bf7355eb3 100644 --- a/packages/react-devtools-extensions/webpack.config.js +++ b/packages/react-devtools-extensions/webpack.config.js @@ -109,6 +109,11 @@ module.exports = { }, ], }, + { + test: /\.worker\.js$/, + // inline: true due to limitations with extensions + use: {loader: 'workerize-loader', options: {inline: true}}, + }, ], }, }; diff --git a/packages/react-devtools-shared/src/hookNamesCache.js b/packages/react-devtools-shared/src/hookNamesCache.js index f385c72428525..070dafeb1d8eb 100644 --- a/packages/react-devtools-shared/src/hookNamesCache.js +++ b/packages/react-devtools-shared/src/hookNamesCache.js @@ -18,7 +18,7 @@ import type { } from 'react-devtools-shared/src/types'; import type {HookSource} from 'react-debug-tools/src/ReactDebugHooks'; -const TIMEOUT = 5000; +const TIMEOUT = 30000; const Pending = 0; const Resolved = 1; diff --git a/yarn.lock b/yarn.lock index c0bddfe82c64e..03daa69ff8127 100644 --- a/yarn.lock +++ b/yarn.lock @@ -14877,6 +14877,13 @@ worker-loader@^3.0.2: loader-utils "^2.0.0" schema-utils "^2.7.0" +workerize-loader@^1.3.0: + version "1.3.0" + resolved "https://registry.yarnpkg.com/workerize-loader/-/workerize-loader-1.3.0.tgz#4995cf2ff2b45dd6dc60e4411e63f5ae2c704d36" + integrity sha512-utWDc8K6embcICmRBUUkzanPgKBb8yM1OHfh6siZfiMsswE8wLCa9CWS+L7AARz0+Th4KH4ZySrqer/OJ9WuWw== + dependencies: + loader-utils "^2.0.0" + wrap-ansi@^2.0.0: version "2.1.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-2.1.0.tgz#d8fc3d284dd05794fe84973caecdd1cf824fdd85"