Skip to content

Commit

Permalink
DevTools: Parse named source AST in a worker (facebook#21902)
Browse files Browse the repository at this point in the history
Resolves facebook#21855

Ended up using workerize in order to setup the worker once it allows easy imports (for babel's parse function) and exports.
  • Loading branch information
tsirlucas authored and zhengjitf committed Apr 15, 2022
1 parent 86b97aa commit 920d99e
Show file tree
Hide file tree
Showing 9 changed files with 161 additions and 45 deletions.
2 changes: 1 addition & 1 deletion packages/react-devtools-extensions/firefox/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 3 additions & 2 deletions packages/react-devtools-extensions/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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);
});
});
33 changes: 33 additions & 0 deletions packages/react-devtools-extensions/src/parseHookNames/index.js
Original file line number Diff line number Diff line change
@@ -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<ParseHookNamesModule, 'parseHookNames'>;

export const parseHookNames: ParseHookNames = hooksTree =>
workerizedParseHookNames.parseHookNames(hooksTree, wasmMappingsURL);

export const purgeCachedMetadata = workerizedParseHookNames.purgeCachedMetadata;
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
/* global chrome */

/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
Expand All @@ -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';

Expand All @@ -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;
Expand Down Expand Up @@ -103,6 +101,7 @@ const originalURLToMetadataCache: LRUCache<

export async function parseHookNames(
hooksTree: HooksTree,
wasmMappingsURL: string,
): Thenable<HookNames | null> {
const hooksList: Array<HooksNode> = [];
flattenHooksList(hooksTree, hooksList);
Expand Down Expand Up @@ -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));
Expand All @@ -182,6 +183,7 @@ function decodeBase64String(encoded: string): Object {

function extractAndLoadSourceMaps(
locationKeyToHookSourceData: Map<string, HookSourceData>,
wasmMappingsURL: string,
): Promise<*> {
// SourceMapConsumer.initialize() does nothing when running in Node (aka Jest)
// because the wasm file is automatically read from the file system
Expand All @@ -193,9 +195,6 @@ function extractAndLoadSourceMaps(
);
}

// $FlowFixMe
const wasmMappingsURL = chrome.extension.getURL('mappings.wasm');

SourceMapConsumer.initialize({'lib/mappings.wasm': wasmMappingsURL});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import * as parseHookNamesModule from './parseHookNames';

export const parseHookNames = parseHookNamesModule.parseHookNames;
export const purgeCachedMetadata = parseHookNamesModule.purgeCachedMetadata;
5 changes: 5 additions & 0 deletions packages/react-devtools-extensions/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ module.exports = {
},
],
},
{
test: /\.worker\.js$/,
// inline: true due to limitations with extensions
use: {loader: 'workerize-loader', options: {inline: true}},
},
],
},
};
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/hookNamesCache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 920d99e

Please sign in to comment.