diff --git a/packages/react-devtools-cdt-mcp/README.md b/packages/react-devtools-cdt-mcp/README.md index 3229016e11d..47574d7f0d8 100644 --- a/packages/react-devtools-cdt-mcp/README.md +++ b/packages/react-devtools-cdt-mcp/README.md @@ -3,21 +3,29 @@ Integrates React tools with [chrome-devtools-mcp](https://github.com/ChromeDevTools/chrome-devtools-mcp). -Importing this package **before React** installs the DevTools hook and registers -a React tool group via chrome-devtools-mcp's `devtoolstooldiscovery` / `__dtmcp` -third-party-tool protocol. The React tools then become discoverable and callable -inside a chrome-devtools-mcp session — no separate server. +Importing `react-devtools-cdt-mcp/register` **before React** installs the +DevTools hook and registers a React tool group via chrome-devtools-mcp's +`devtoolstooldiscovery` / `__dtmcp` third-party-tool protocol. The React tools +then become discoverable and callable inside a chrome-devtools-mcp session — no +separate server. ## Usage -Import the package **before** React so the hook is installed before React -initializes: +Import the register entry **before** React so the hook is installed before +React initializes: ```js -import 'react-devtools-cdt-mcp'; +import 'react-devtools-cdt-mcp/register'; import React from 'react'; ``` +The package root is side-effect-free and exports the lower-level API for custom +targets: + +```js +import {register, buildToolGroup} from 'react-devtools-cdt-mcp'; +``` + When the page runs under chrome-devtools-mcp, the React tools are listed by `list_3p_developer_tools` and callable either via `execute_3p_developer_tool({toolName, params})` or directly via `evaluate_script` diff --git a/packages/react-devtools-cdt-mcp/fixtures/app/index.js b/packages/react-devtools-cdt-mcp/fixtures/app/index.js index 4c5a99d7ed1..f0f21c3004d 100644 --- a/packages/react-devtools-cdt-mcp/fixtures/app/index.js +++ b/packages/react-devtools-cdt-mcp/fixtures/app/index.js @@ -1,7 +1,7 @@ -// Import the package source first so it installs the DevTools hook and registers -// the chrome-devtools-mcp tool group BEFORE react-dom evaluates — the hook must -// be in place when React's renderer injects on the first commit. -import '../../src/index.js'; +// Import the register entry first so it installs the DevTools hook and +// registers the chrome-devtools-mcp tool group BEFORE react-dom evaluates — the +// hook must be in place when React's renderer injects on the first commit. +import '../../src/register.js'; import * as React from 'react'; import {createRoot} from 'react-dom/client'; diff --git a/packages/react-devtools-cdt-mcp/package.json b/packages/react-devtools-cdt-mcp/package.json index b5988d68249..59bf0a6fc7e 100644 --- a/packages/react-devtools-cdt-mcp/package.json +++ b/packages/react-devtools-cdt-mcp/package.json @@ -11,7 +11,8 @@ }, "files": [ "dist", - "index.js" + "index.js", + "register.js" ], "scripts": { "build": "cross-env NODE_ENV=production rollup -c rollup.config.cjs", diff --git a/packages/react-devtools-cdt-mcp/register.js b/packages/react-devtools-cdt-mcp/register.js new file mode 100644 index 00000000000..25157cb98d3 --- /dev/null +++ b/packages/react-devtools-cdt-mcp/register.js @@ -0,0 +1,3 @@ +'use strict'; + +module.exports = require('./dist/register.js'); diff --git a/packages/react-devtools-cdt-mcp/rollup.config.cjs b/packages/react-devtools-cdt-mcp/rollup.config.cjs index 50cc2037973..01e343db937 100644 --- a/packages/react-devtools-cdt-mcp/rollup.config.cjs +++ b/packages/react-devtools-cdt-mcp/rollup.config.cjs @@ -9,35 +9,44 @@ if (!NODE_ENV) { process.exit(1); } -module.exports = { - input: 'src/index.js', - // CommonJS bundle for the npm package (referenced by the root index.js stub). - output: { - file: 'dist/index.js', - format: 'cjs', - exports: 'named', - }, - treeshake: {moduleSideEffects: false}, - plugins: [ - nodeResolve({ - extensions: ['.js', '.mjs'], - }), - commonjs({ - include: /node_modules/, - }), - babel({ - configFile: __dirname + '/../react-devtools-shared/babel.config.js', - babelHelpers: 'bundled', - }), - replace({ - preventAssignment: true, - values: { - __DEV__: String(NODE_ENV === 'development'), - __IS_CHROME__: 'false', - __IS_FIREFOX__: 'false', - __IS_EDGE__: 'false', - __IS_NATIVE__: 'false', - }, - }), - ], -}; +const plugins = [ + nodeResolve({ + extensions: ['.js', '.mjs'], + }), + commonjs({ + include: /node_modules/, + }), + babel({ + configFile: __dirname + '/../react-devtools-shared/babel.config.js', + babelHelpers: 'bundled', + }), + replace({ + preventAssignment: true, + values: { + __DEV__: String(NODE_ENV === 'development'), + __IS_CHROME__: 'false', + __IS_FIREFOX__: 'false', + __IS_EDGE__: 'false', + __IS_NATIVE__: 'false', + }, + }), +]; + +function createBundle(input, file) { + return { + input, + // CommonJS bundles for the npm package (referenced by root stubs). + output: { + file, + format: 'cjs', + exports: 'named', + }, + treeshake: {moduleSideEffects: false}, + plugins, + }; +} + +module.exports = [ + createBundle('src/index.js', 'dist/index.js'), + createBundle('src/register.js', 'dist/register.js'), +]; diff --git a/packages/react-devtools-cdt-mcp/src/DevToolsCdtMcp.js b/packages/react-devtools-cdt-mcp/src/DevToolsCdtMcp.js index 7277e1039f7..9a3356e334d 100644 --- a/packages/react-devtools-cdt-mcp/src/DevToolsCdtMcp.js +++ b/packages/react-devtools-cdt-mcp/src/DevToolsCdtMcp.js @@ -307,6 +307,17 @@ export type CdtMcpToolGroup = { tools: Array, }; +type Registration = { + facade: Facade, + unregister: () => void, +}; + +type ToolDiscoveryEvent = { + respondWith: (toolGroup: CdtMcpToolGroup) => void, +}; + +const registrations: WeakMap = new WeakMap(); + /** * Build the chrome-devtools-mcp tool group from an assembled set of facade * tools. Each tool returns its facade result directly. @@ -350,10 +361,15 @@ export function register(target?: any = globalThis): { facade: Facade, unregister: () => void, } { + const existingRegistration: Registration | void = registrations.get(target); + if (existingRegistration !== undefined) { + return existingRegistration; + } + const facade = installFacade(target); let toolGroup: CdtMcpToolGroup | null = null; - const listener = (event: any) => { + const listener = (event: ToolDiscoveryEvent) => { if (toolGroup === null) { toolGroup = buildToolGroup(createTools(facade)); } @@ -361,10 +377,19 @@ export function register(target?: any = globalThis): { }; target.addEventListener('devtoolstooldiscovery', listener); - return { + let isRegistered = true; + const registration: Registration = { facade, unregister: () => { + if (!isRegistered) { + return; + } + isRegistered = false; target.removeEventListener('devtoolstooldiscovery', listener); + registrations.delete(target); }, }; + + registrations.set(target, registration); + return registration; } diff --git a/packages/react-devtools-cdt-mcp/src/__tests__/DevToolsCdtMcp-test.js b/packages/react-devtools-cdt-mcp/src/__tests__/DevToolsCdtMcp-test.js index c2db824fb87..8f291699b52 100644 --- a/packages/react-devtools-cdt-mcp/src/__tests__/DevToolsCdtMcp-test.js +++ b/packages/react-devtools-cdt-mcp/src/__tests__/DevToolsCdtMcp-test.js @@ -94,6 +94,165 @@ describe('react-devtools-cdt-mcp', () => { expect(globalThis.__dtmcp).toBeUndefined(); }); + it('root entry exports tools without registering', () => { + unregister(); + delete globalThis.__REACT_DEVTOOLS_GLOBAL_HOOK__; + jest.resetModules(); + + const api = require('../index'); + + expect(typeof api.register).toBe('function'); + expect(typeof api.buildToolGroup).toBe('function'); + expect(globalThis.__REACT_DEVTOOLS_GLOBAL_HOOK__).toBeUndefined(); + }); + + it('throws when the register entry is imported outside an event target', () => { + const originalAddEventListener = globalThis.addEventListener; + const originalRemoveEventListener = globalThis.removeEventListener; + + unregister(); + delete globalThis.__REACT_DEVTOOLS_GLOBAL_HOOK__; + jest.resetModules(); + + try { + // $FlowFixMe[cannot-write] + globalThis.addEventListener = undefined; + // $FlowFixMe[cannot-write] + globalThis.removeEventListener = undefined; + + expect(() => require('../register')).toThrow( + 'react-devtools-cdt-mcp/register must be imported in a browser-like environment', + ); + expect(globalThis.__REACT_DEVTOOLS_GLOBAL_HOOK__).toBeUndefined(); + } finally { + globalThis.addEventListener = originalAddEventListener; + globalThis.removeEventListener = originalRemoveEventListener; + } + }); + + it('register entry installs the DevTools hook', () => { + const originalAddEventListener = globalThis.addEventListener; + const originalRemoveEventListener = globalThis.removeEventListener; + let autoListener = null; + + unregister(); + delete globalThis.__REACT_DEVTOOLS_GLOBAL_HOOK__; + jest.resetModules(); + + try { + // $FlowFixMe[cannot-write] + globalThis.addEventListener = (type, listener, options) => { + if (type === 'devtoolstooldiscovery') { + autoListener = listener; + } + return originalAddEventListener.call( + globalThis, + type, + listener, + options, + ); + }; + + require('../register'); + + expect(globalThis.__REACT_DEVTOOLS_GLOBAL_HOOK__).toBeDefined(); + } finally { + if (autoListener !== null) { + originalRemoveEventListener.call( + globalThis, + 'devtoolstooldiscovery', + autoListener, + ); + } + globalThis.addEventListener = originalAddEventListener; + globalThis.removeEventListener = originalRemoveEventListener; + } + }); + + it('returns the cached registration for repeated calls per target', () => { + let listener = null; + const target = { + addEventListener: jest.fn((type, callback) => { + expect(type).toBe('devtoolstooldiscovery'); + listener = callback; + }), + removeEventListener: jest.fn(), + }; + + const first = register(target); + const second = register(target); + + expect(target.addEventListener).toHaveBeenCalledTimes(1); + expect(second).toBe(first); + expect(second.facade).toBe(first.facade); + + let firstGroup = null; + let secondGroup = null; + listener({ + respondWith: group => { + firstGroup = group; + }, + }); + listener({ + respondWith: group => { + secondGroup = group; + }, + }); + expect(secondGroup).toBe(firstGroup); + + first.unregister(); + expect(target.removeEventListener).toHaveBeenCalledTimes(1); + expect(target.removeEventListener).toHaveBeenCalledWith( + 'devtoolstooldiscovery', + listener, + ); + + second.unregister(); + expect(target.removeEventListener).toHaveBeenCalledTimes(1); + + const third = register(target); + expect(third).not.toBe(first); + expect(target.addEventListener).toHaveBeenCalledTimes(2); + third.unregister(); + }); + + it('does not write registration state to the target', () => { + let listener = null; + const existingHook = { + inject: jest.fn(() => 0), + onCommitFiberRoot: jest.fn(), + onPostCommitFiberRoot: jest.fn(), + renderers: new Map(), + }; + const target = Object.preventExtensions({ + __REACT_DEVTOOLS_GLOBAL_HOOK__: existingHook, + addEventListener: jest.fn((type, callback) => { + expect(type).toBe('devtoolstooldiscovery'); + listener = callback; + }), + removeEventListener: jest.fn(), + }); + + const first = register(target); + const second = register(target); + + expect(target.addEventListener).toHaveBeenCalledTimes(1); + expect(second).toBe(first); + expect(second.facade).toBe(first.facade); + expect(Object.keys(target).sort()).toEqual([ + '__REACT_DEVTOOLS_GLOBAL_HOOK__', + 'addEventListener', + 'removeEventListener', + ]); + + first.unregister(); + second.unregister(); + expect(target.removeEventListener).toHaveBeenCalledWith( + 'devtoolstooldiscovery', + listener, + ); + }); + it('builds a "react" tool group exposing every facade tool', () => { expect(toolGroup.name).toBe('react'); expect(typeof toolGroup.description).toBe('string'); diff --git a/packages/react-devtools-cdt-mcp/src/index.js b/packages/react-devtools-cdt-mcp/src/index.js index c6e0887590d..e383df67e0a 100644 --- a/packages/react-devtools-cdt-mcp/src/index.js +++ b/packages/react-devtools-cdt-mcp/src/index.js @@ -7,10 +7,4 @@ * @flow */ -import {register} from './DevToolsCdtMcp'; - -// Side effect: install the facade (before React) and register the React tool -// group for chrome-devtools-mcp. Import this module before React. -register(); - export * from './DevToolsCdtMcp'; diff --git a/packages/react-devtools-cdt-mcp/src/register.js b/packages/react-devtools-cdt-mcp/src/register.js new file mode 100644 index 00000000000..423375fc74c --- /dev/null +++ b/packages/react-devtools-cdt-mcp/src/register.js @@ -0,0 +1,29 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import {register} from './DevToolsCdtMcp'; + +// Side effect: install the facade (before React) and register the React tool +// group for chrome-devtools-mcp. Import this module before React. +if ( + typeof window !== 'undefined' && + typeof window.addEventListener === 'function' && + typeof window.removeEventListener === 'function' +) { + register(); +} else { + // eslint-disable-next-line react-internal/prod-error-codes + throw new Error( + 'react-devtools-cdt-mcp/register must be imported in a browser-like ' + + 'environment before React initializes. Use the root entry point for ' + + 'side-effect-free access to register().', + ); +} + +export * from './DevToolsCdtMcp'; diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index 557020620e9..91b3586b816 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -608,6 +608,7 @@ export default class Store extends EventEmitter<{ root = this._idToElement.get(rootID); if (root === undefined) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error( `Couldn't find root with id "${rootID}": no matching node was found in the Store.`, @@ -644,6 +645,7 @@ export default class Store extends EventEmitter<{ const child = this._idToElement.get(childID); if (child === undefined) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error( `Couldn't child element with id "${childID}": no matching node was found in the Store.`, @@ -1431,6 +1433,7 @@ export default class Store extends EventEmitter<{ i += 3; if (this._idToElement.has(id)) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error( `Cannot add node "${id}" because a node with that id is already in the Store.`, @@ -1541,6 +1544,7 @@ export default class Store extends EventEmitter<{ const parentElement = this._idToElement.get(parentID); if (parentElement === undefined) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error( `Cannot add child "${id}" to parent "${parentID}" because parent node was not found in the Store.`, @@ -1617,6 +1621,7 @@ export default class Store extends EventEmitter<{ const element = this._idToElement.get(id); if (element === undefined) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error( `Cannot remove node "${id}" because no matching node was found in the Store.`, @@ -1630,6 +1635,7 @@ export default class Store extends EventEmitter<{ const {children, ownerID, parentID, weight} = element; if (children.length > 0) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error(`Node "${id}" was removed before its children.`), ); @@ -1657,6 +1663,7 @@ export default class Store extends EventEmitter<{ parentElement = this._idToElement.get(parentID); if (parentElement === undefined) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error( `Cannot remove node "${id}" from parent "${parentID}" because no matching node was found in the Store.`, @@ -1696,6 +1703,7 @@ export default class Store extends EventEmitter<{ const element = this._idToElement.get(id); if (element === undefined) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error( `Cannot reorder children for node "${id}" because no matching node was found in the Store.`, @@ -1707,6 +1715,7 @@ export default class Store extends EventEmitter<{ const children = element.children; if (children.length !== numChildren) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error( `Children cannot be added or removed during a reorder operation.`, @@ -1824,6 +1833,7 @@ export default class Store extends EventEmitter<{ let name = stringTable[nameStringID]; if (this._idToSuspense.has(id)) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error( `Cannot add suspense node "${id}" because a suspense node with that id is already in the Store.`, @@ -1876,6 +1886,7 @@ export default class Store extends EventEmitter<{ if (parentID !== 0) { const parentSuspense = this._idToSuspense.get(parentID); if (parentSuspense === undefined) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error( `Cannot add suspense child "${id}" to parent suspense "${parentID}" because parent suspense node was not found in the Store.`, @@ -1912,6 +1923,7 @@ export default class Store extends EventEmitter<{ const suspense = this._idToSuspense.get(id); if (suspense === undefined) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error( `Cannot remove suspense node "${id}" because no matching node was found in the Store.`, @@ -1925,6 +1937,7 @@ export default class Store extends EventEmitter<{ const {children, parentID, rects} = suspense; if (children.length > 0) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error(`Suspense node "${id}" was removed before its children.`), ); @@ -1954,6 +1967,7 @@ export default class Store extends EventEmitter<{ parentSuspense = this._idToSuspense.get(parentID); if (parentSuspense === undefined) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error( `Cannot remove suspense node "${id}" from parent "${parentID}" because no matching node was found in the Store.`, @@ -1965,6 +1979,7 @@ export default class Store extends EventEmitter<{ const index = parentSuspense.children.indexOf(id); if (index === -1) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error( `Cannot remove suspense node "${id}" from parent "${parentID}" because it is not a child of the parent.`, @@ -1985,6 +2000,7 @@ export default class Store extends EventEmitter<{ const suspense = this._idToSuspense.get(id); if (suspense === undefined) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error( `Cannot reorder children for suspense node "${id}" because no matching node was found in the Store.`, @@ -1996,6 +2012,7 @@ export default class Store extends EventEmitter<{ const children = suspense.children; if (children.length !== numChildren) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error( `Suspense children cannot be added or removed during a reorder operation.`, @@ -2036,6 +2053,7 @@ export default class Store extends EventEmitter<{ const suspense = this._idToSuspense.get(id); if (suspense === undefined) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error( `Cannot set rects for suspense node "${id}" because no matching node was found in the Store.`, @@ -2123,6 +2141,7 @@ export default class Store extends EventEmitter<{ const suspense = this._idToSuspense.get(id); if (suspense === undefined) { + // We should never reach this. This is a bug in the backend renderer. this._throwAndEmitError( Error( `Cannot update suspenders of suspense node "${id}" because no matching node was found in the Store.`, diff --git a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js index 2ba8ca79f5f..ee30b8034d8 100644 --- a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js +++ b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js @@ -42,7 +42,11 @@ export function getValueForAttribute( } return expected === undefined ? undefined : null; } - const value = node.getAttribute(name); + // When CSP is enabled, browsers hide the nonce attribute + // so we need to access the nonce property directly + // https://html.spec.whatwg.org/multipage/urls-and-fetching.html#cryptographicnonce + const isNonce = name.toLowerCase() === 'nonce'; + const value = isNonce ? (node as any).nonce : node.getAttribute(name); if (__DEV__) { checkAttributeStringCoercion(expected, name); } @@ -79,7 +83,11 @@ export function getValueForAttributeOnCustomComponent( } return expected === undefined ? undefined : null; } - const value = node.getAttribute(name); + // When CSP is enabled, browsers hide the nonce attribute + // so we need to access the nonce property directly + // https://html.spec.whatwg.org/multipage/urls-and-fetching.html#cryptographicnonce + const isNonce = name.toLowerCase() === 'nonce'; + const value = isNonce ? (node as any).nonce : node.getAttribute(name); if (value === '' && expected === true) { return true; diff --git a/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js b/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js index 835ffaac79d..40668f04529 100644 --- a/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js @@ -47,6 +47,7 @@ describe('ReactDOMServerHydration', () => { }); afterEach(() => { + jest.restoreAllMocks(); window.removeEventListener('error', errorHandler); document.body.removeChild(container); console.error = realConsoleError; @@ -525,6 +526,74 @@ describe('ReactDOMServerHydration', () => { ] `); }); + + describe('nonce', () => { + // Nonce is on HTMLOrSVGElement, so cover a few host tags that hydrate + // attributes through getValueForAttribute. + function App() { + return ( +
+