From fecc288b7dce182fbc3056b3f98a445a95fb1e68 Mon Sep 17 00:00:00 2001 From: Robert Balicki Date: Tue, 25 Oct 2022 13:25:33 +1100 Subject: [PATCH 01/44] [react devtools] Device storage support (#25452) # Summary * This PR adds support for persisting certain settings to device storage, allowing e.g. RN apps to properly patch the console when restarted. * The device storage APIs have signature `getConsolePatchSettings()` and `setConsolePatchSettings(string)`, in iOS, are thin wrappers around the `Library/Settings` turbomodule, and wrap a new TM that uses the `SharedPreferences` class in Android. * Pass device storage getters/setters from RN to DevTools' `connectToDevtools`. The setters are then used to populate values on `window`. Later, the console is patched using these values. * If we receive a notification from DevTools that the console patching fields have been updated, we write values back to local storage. * See https://github.com/facebook/react-native/pull/34903 # How did you test this change? Manual testing, `yarn run test-build-devtools`, `yarn run prettier`, `yarn run flow dom` ## Manual testing setup: ### React DevTools Frontend * Get the DevTools frontend in flipper: * `nvm install -g react-devtools-core`, then replace that package with a symlink to the local package * enable "use globally installed devtools" in flipper * yarn run start in react-devtools, etc. as well ### React DevTools Backend * `yarn run build:backend` in react-devtools-core, then copy-paste that file to the expo app's node_modules directory ### React Native * A local version of React Native can be patched in by modifying an expo app's package.json, as in `"react-native": "rbalicki2/react-native#branch-name"` # Versioning safety * There are three versioned modules to worry about: react native, the devtools frontend and the devtools backend. * The react devtools backend checks for whether a `cachedSettingsStore` is passed from react native. If not (e.g. if React Native is outdated), then no behavior changes. * The devtools backend reads the patched console values from the cached settings store. However, if nothing has been stored, for example because the frontend is outdated or has never synced its settings, then behavior doesn't change. * The devtools frontend sends no new messages. However, if it did send a new message (e.g. "store this value at this key"), and the backend was outdated, that message would be silently ignored. --- packages/react-devtools-core/src/backend.js | 26 +++++++ .../react-devtools-core/src/cachedSettings.js | 77 +++++++++++++++++++ .../src/backend/agent.js | 10 +-- .../src/backend/console.js | 42 ++++++---- packages/react-devtools-shared/src/bridge.js | 12 +-- packages/react-devtools-shared/src/utils.js | 13 ++++ 6 files changed, 148 insertions(+), 32 deletions(-) create mode 100644 packages/react-devtools-core/src/cachedSettings.js diff --git a/packages/react-devtools-core/src/backend.js b/packages/react-devtools-core/src/backend.js index bd17e6fba4b02..70048a9de6cd1 100644 --- a/packages/react-devtools-core/src/backend.js +++ b/packages/react-devtools-core/src/backend.js @@ -14,6 +14,11 @@ import {initBackend} from 'react-devtools-shared/src/backend'; import {__DEBUG__} from 'react-devtools-shared/src/constants'; import setupNativeStyleEditor from 'react-devtools-shared/src/backend/NativeStyleEditor/setupNativeStyleEditor'; import {getDefaultComponentFilters} from 'react-devtools-shared/src/utils'; +import { + initializeUsingCachedSettings, + cacheConsolePatchSettings, + type DevToolsSettingsManager, +} from './cachedSettings'; import type {BackendBridge} from 'react-devtools-shared/src/bridge'; import type {ComponentFilter} from 'react-devtools-shared/src/types'; @@ -29,6 +34,7 @@ type ConnectOptions = { retryConnectionDelay?: number, isAppActive?: () => boolean, websocket?: ?WebSocket, + devToolsSettingsManager: ?DevToolsSettingsManager, ... }; @@ -63,6 +69,7 @@ export function connectToDevTools(options: ?ConnectOptions) { resolveRNStyle = null, retryConnectionDelay = 2000, isAppActive = () => true, + devToolsSettingsManager, } = options || {}; const protocol = useHttps ? 'wss' : 'ws'; @@ -78,6 +85,16 @@ export function connectToDevTools(options: ?ConnectOptions) { } } + if (devToolsSettingsManager != null) { + try { + initializeUsingCachedSettings(devToolsSettingsManager); + } catch (e) { + // If we call a method on devToolsSettingsManager that throws, or if + // is invalid data read out, don't throw and don't interrupt initialization + console.error(e); + } + } + if (!isAppActive()) { // If the app is in background, maybe retry later. // Don't actually attempt to connect until we're in foreground. @@ -142,6 +159,15 @@ export function connectToDevTools(options: ?ConnectOptions) { }, ); + if (devToolsSettingsManager != null && bridge != null) { + bridge.addListener('updateConsolePatchSettings', consolePatchSettings => + cacheConsolePatchSettings( + devToolsSettingsManager, + consolePatchSettings, + ), + ); + } + // The renderer interface doesn't read saved component filters directly, // because they are generally stored in localStorage within the context of the extension. // Because of this it relies on the extension to pass filters. diff --git a/packages/react-devtools-core/src/cachedSettings.js b/packages/react-devtools-core/src/cachedSettings.js new file mode 100644 index 0000000000000..cb6dddfec313b --- /dev/null +++ b/packages/react-devtools-core/src/cachedSettings.js @@ -0,0 +1,77 @@ +/** + * 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 + */ + +import { + type ConsolePatchSettings, + writeConsolePatchSettingsToWindow, +} from 'react-devtools-shared/src/backend/console'; +import {castBool, castBrowserTheme} from 'react-devtools-shared/src/utils'; + +// Note: all keys should be optional in this type, because users can use newer +// versions of React DevTools with older versions of React Native, and the object +// provided by React Native may not include all of this type's fields. +export type DevToolsSettingsManager = { + getConsolePatchSettings: ?() => string, + setConsolePatchSettings: ?(key: string) => void, +}; + +export function initializeUsingCachedSettings( + devToolsSettingsManager: DevToolsSettingsManager, +) { + initializeConsolePatchSettings(devToolsSettingsManager); +} + +function initializeConsolePatchSettings( + devToolsSettingsManager: DevToolsSettingsManager, +) { + if (devToolsSettingsManager.getConsolePatchSettings == null) { + return; + } + const consolePatchSettingsString = devToolsSettingsManager.getConsolePatchSettings(); + if (consolePatchSettingsString == null) { + return; + } + const parsedConsolePatchSettings = parseConsolePatchSettings( + consolePatchSettingsString, + ); + if (parsedConsolePatchSettings == null) { + return; + } + writeConsolePatchSettingsToWindow(parsedConsolePatchSettings); +} + +function parseConsolePatchSettings( + consolePatchSettingsString: string, +): ?ConsolePatchSettings { + const parsedValue = JSON.parse(consolePatchSettingsString ?? '{}'); + const { + appendComponentStack, + breakOnConsoleErrors, + showInlineWarningsAndErrors, + hideConsoleLogsInStrictMode, + browserTheme, + } = parsedValue; + return { + appendComponentStack: castBool(appendComponentStack) ?? true, + breakOnConsoleErrors: castBool(breakOnConsoleErrors) ?? false, + showInlineWarningsAndErrors: castBool(showInlineWarningsAndErrors) ?? true, + hideConsoleLogsInStrictMode: castBool(hideConsoleLogsInStrictMode) ?? false, + browserTheme: castBrowserTheme(browserTheme) ?? 'dark', + }; +} + +export function cacheConsolePatchSettings( + devToolsSettingsManager: DevToolsSettingsManager, + value: ConsolePatchSettings, +): void { + if (devToolsSettingsManager.setConsolePatchSettings == null) { + return; + } + devToolsSettingsManager.setConsolePatchSettings(JSON.stringify(value)); +} diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js index 1a3ea52e45b32..78812286bf211 100644 --- a/packages/react-devtools-shared/src/backend/agent.js +++ b/packages/react-devtools-shared/src/backend/agent.js @@ -25,7 +25,7 @@ import { initialize as setupTraceUpdates, toggleEnabled as setTraceUpdatesEnabled, } from './views/TraceUpdates'; -import {patch as patchConsole} from './console'; +import {patch as patchConsole, type ConsolePatchSettings} from './console'; import {currentBridgeProtocol} from 'react-devtools-shared/src/bridge'; import type {BackendBridge} from 'react-devtools-shared/src/bridge'; @@ -712,11 +712,11 @@ export default class Agent extends EventEmitter<{ showInlineWarningsAndErrors, hideConsoleLogsInStrictMode, browserTheme, - }) => { - // If the frontend preference has change, - // or in the case of React Native- if the backend is just finding out the preference- + }: ConsolePatchSettings) => { + // If the frontend preferences have changed, + // or in the case of React Native- if the backend is just finding out the preferences- // then reinstall the console overrides. - // It's safe to call these methods multiple times, so we don't need to worry about that. + // It's safe to call `patchConsole` multiple times. patchConsole({ appendComponentStack, breakOnConsoleErrors, diff --git a/packages/react-devtools-shared/src/backend/console.js b/packages/react-devtools-shared/src/backend/console.js index 274c0ba083969..5753d910eba1d 100644 --- a/packages/react-devtools-shared/src/backend/console.js +++ b/packages/react-devtools-shared/src/backend/console.js @@ -15,6 +15,7 @@ import {format, formatWithStyles} from './utils'; import {getInternalReactConstants} from './renderer'; import {getStackByFiberInDevAndProd} from './DevToolsFiberComponentStack'; import {consoleManagedByDevToolsDuringStrictMode} from 'react-devtools-feature-flags'; +import {castBool, castBrowserTheme} from '../utils'; const OVERRIDE_CONSOLE_METHODS = ['error', 'trace', 'warn']; const DIMMED_NODE_CONSOLE_COLOR = '\x1b[2m%s\x1b[0m'; @@ -143,6 +144,14 @@ const consoleSettingsRef = { browserTheme: 'dark', }; +export type ConsolePatchSettings = { + appendComponentStack: boolean, + breakOnConsoleErrors: boolean, + showInlineWarningsAndErrors: boolean, + hideConsoleLogsInStrictMode: boolean, + browserTheme: BrowserTheme, +}; + // Patches console methods to append component stack for the current fiber. // Call unpatch() to remove the injected behavior. export function patch({ @@ -151,13 +160,7 @@ export function patch({ showInlineWarningsAndErrors, hideConsoleLogsInStrictMode, browserTheme, -}: { - appendComponentStack: boolean, - breakOnConsoleErrors: boolean, - showInlineWarningsAndErrors: boolean, - hideConsoleLogsInStrictMode: boolean, - browserTheme: BrowserTheme, -}): void { +}: ConsolePatchSettings): void { // Settings may change after we've patched the console. // Using a shared ref allows the patch function to read the latest values. consoleSettingsRef.appendComponentStack = appendComponentStack; @@ -390,14 +393,19 @@ export function patchConsoleUsingWindowValues() { }); } -function castBool(v: any): ?boolean { - if (v === true || v === false) { - return v; - } -} - -function castBrowserTheme(v: any): ?BrowserTheme { - if (v === 'light' || v === 'dark' || v === 'auto') { - return v; - } +// After receiving cached console patch settings from React Native, we set them on window. +// When the console is initially patched (in renderer.js and hook.js), these values are read. +// The browser extension (etc.) sets these values on window, but through another method. +export function writeConsolePatchSettingsToWindow( + settings: ConsolePatchSettings, +): void { + window.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = + settings.appendComponentStack; + window.__REACT_DEVTOOLS_BREAK_ON_CONSOLE_ERRORS__ = + settings.breakOnConsoleErrors; + window.__REACT_DEVTOOLS_SHOW_INLINE_WARNINGS_AND_ERRORS__ = + settings.showInlineWarningsAndErrors; + window.__REACT_DEVTOOLS_HIDE_CONSOLE_LOGS_IN_STRICT_MODE__ = + settings.hideConsoleLogsInStrictMode; + window.__REACT_DEVTOOLS_BROWSER_THEME__ = settings.browserTheme; } diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index 64b32a44483cb..19549e1b0dbe2 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -17,7 +17,7 @@ import type { RendererID, } from 'react-devtools-shared/src/backend/types'; import type {StyleAndLayout as StyleAndLayoutPayload} from 'react-devtools-shared/src/backend/NativeStyleEditor/types'; -import type {BrowserTheme} from 'react-devtools-shared/src/devtools/views/DevTools'; +import type {ConsolePatchSettings} from 'react-devtools-shared/src/backend/console'; const BATCH_DURATION = 100; @@ -171,14 +171,6 @@ type NativeStyleEditor_SetValueParams = { value: string, }; -type UpdateConsolePatchSettingsParams = { - appendComponentStack: boolean, - breakOnConsoleErrors: boolean, - showInlineWarningsAndErrors: boolean, - hideConsoleLogsInStrictMode: boolean, - browserTheme: BrowserTheme, -}; - type SavedPreferencesParams = { appendComponentStack: boolean, breakOnConsoleErrors: boolean, @@ -247,7 +239,7 @@ type FrontendEvents = { stopProfiling: [], storeAsGlobal: [StoreAsGlobalParams], updateComponentFilters: [Array], - updateConsolePatchSettings: [UpdateConsolePatchSettingsParams], + updateConsolePatchSettings: [ConsolePatchSettings], viewAttributeSource: [ViewAttributeSourceParams], viewElementSource: [ElementAndRendererID], diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index a896b764aa24e..dd76f4b723a70 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -55,6 +55,7 @@ import isArray from './isArray'; import type {ComponentFilter, ElementType} from './types'; import type {LRUCache} from 'react-devtools-shared/src/types'; +import type {BrowserTheme} from 'react-devtools-shared/src/devtools/views/DevTools'; // $FlowFixMe[method-unbinding] const hasOwnProperty = Object.prototype.hasOwnProperty; @@ -353,6 +354,18 @@ function parseBool(s: ?string): ?boolean { } } +export function castBool(v: any): ?boolean { + if (v === true || v === false) { + return v; + } +} + +export function castBrowserTheme(v: any): ?BrowserTheme { + if (v === 'light' || v === 'dark' || v === 'auto') { + return v; + } +} + export function getAppendComponentStack(): boolean { const raw = localStorageGetItem( LOCAL_STORAGE_SHOULD_APPEND_COMPONENT_STACK_KEY, From 17204056d5ec0a53a74979321c519137d215cf64 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 26 Oct 2022 19:53:29 -0700 Subject: [PATCH 02/44] [Float] fix coordination of resource identity and hydration (#25569) there are a few bugs where dom representations from SSR aren't identified as Resources when they should be. There are 3 semantics Resource -> hoist to head, deduping, etc... hydratable Component -> SSR'd and hydrated in place non-hydratable Component -> never SSR'd, never hydrated, always inserted on the client this last category is small (non stylesheet) links with onLoad and/or onError async scripts with onLoad and/or onError The reason we have this distinction for now is we need every SSR'd async script to be assumable to be a Resource. we don't currently encode onLoad on the server and so we couldn't otherwise tell if an async script is a Resource or is an async script with an onLoad which would not be a resource. To avoid this ambiguity we never emit the scripts in SSR and assume they need to be inserted on the client. We can explore changes to these semantics in the future or possibly encode some identifier when we want to opt out of resource semantics but still SSR the link or script. --- .../src/client/ReactDOMFloatClient.js | 4 +- .../src/client/ReactDOMHostConfig.js | 52 +++++++++++------ .../src/server/ReactDOMFloatServer.js | 2 +- .../src/__tests__/ReactDOMFloat-test.js | 58 +++++++++++++++++++ 4 files changed, 94 insertions(+), 22 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMFloatClient.js b/packages/react-dom-bindings/src/client/ReactDOMFloatClient.js index 97b222d080d4e..c9e8607eaaf7d 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMFloatClient.js +++ b/packages/react-dom-bindings/src/client/ReactDOMFloatClient.js @@ -1465,9 +1465,9 @@ export function isHostResourceType(type: string, props: Props): boolean { } return (async: any) && typeof src === 'string' && !onLoad && !onError; } + case 'noscript': case 'template': - case 'style': - case 'noscript': { + case 'style': { if (__DEV__) { if (resourceFormOnly) { console.error( diff --git a/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js b/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js index cfd3131e9ba9a..a4d394a986108 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js @@ -802,7 +802,15 @@ export const supportsHydration = true; // inserted without breaking hydration export function isHydratable(type: string, props: Props): boolean { if (enableFloat) { - if (type === 'script') { + if (type === 'link') { + if ( + (props: any).rel === 'stylesheet' && + typeof (props: any).precedence !== 'string' + ) { + return true; + } + return false; + } else if (type === 'script') { const {async, onLoad, onError} = (props: any); return !(async && (onLoad || onError)); } @@ -902,16 +910,25 @@ function getNextHydratable(node) { if (nodeType === ELEMENT_NODE) { const element: Element = (node: any); switch (element.tagName) { + case 'TITLE': + case 'META': + case 'BASE': + case 'HTML': + case 'HEAD': + case 'BODY': { + continue; + } case 'LINK': { const linkEl: HTMLLinkElement = (element: any); - const rel = linkEl.rel; + // All links that are server rendered are resources except + // stylesheets that do not have a precedence if ( - rel === 'preload' || - (rel === 'stylesheet' && linkEl.hasAttribute('data-precedence')) + linkEl.rel === 'stylesheet' && + !linkEl.hasAttribute('data-precedence') ) { - continue; + break; } - break; + continue; } case 'STYLE': { const styleEl: HTMLStyleElement = (element: any); @@ -927,12 +944,6 @@ function getNextHydratable(node) { } break; } - case 'TITLE': - case 'HTML': - case 'HEAD': - case 'BODY': { - continue; - } } break; } else if (nodeType === TEXT_NODE) { @@ -942,18 +953,21 @@ function getNextHydratable(node) { if (nodeType === ELEMENT_NODE) { const element: Element = (node: any); switch (element.tagName) { + case 'TITLE': + case 'META': + case 'BASE': { + continue; + } case 'LINK': { const linkEl: HTMLLinkElement = (element: any); - const rel = linkEl.rel; + // All links that are server rendered are resources except + // stylesheets that do not have a precedence if ( - rel === 'preload' || - (rel === 'stylesheet' && linkEl.hasAttribute('data-precedence')) + linkEl.rel === 'stylesheet' && + !linkEl.hasAttribute('data-precedence') ) { - continue; + break; } - break; - } - case 'TITLE': { continue; } case 'STYLE': { diff --git a/packages/react-dom-bindings/src/server/ReactDOMFloatServer.js b/packages/react-dom-bindings/src/server/ReactDOMFloatServer.js index ebfedb750254e..46cfe75c95510 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMFloatServer.js +++ b/packages/react-dom-bindings/src/server/ReactDOMFloatServer.js @@ -863,7 +863,7 @@ export function resourcesFromLink(props: Props): boolean { } } if (props.onLoad || props.onError) { - return false; + return true; } const sizes = typeof props.sizes === 'string' ? props.sizes : ''; diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index 61c8d137acd33..7c4e826b22f8c 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -281,6 +281,64 @@ describe('ReactDOMFloat', () => { }); } + // @gate enableFloat + it('can hydrate non Resources in head when Resources are also inserted there', async () => { + await actIntoEmptyDocument(() => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + + + + {}} /> + foo + +