facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
228.04k stars 46.53k forks source link

CRA: Fast Refresh breaks embedded DevTools backend #20377

Open alexanderhupfer opened 3 years ago

alexanderhupfer commented 3 years ago

`TypeError: undefined is not a function (near '...e.sub...') (anonymous function) src/backend/index.js:26 23 | // DevTools didn't get injected into this page (maybe b'c of the contentType). 24 | return () => {}; 25 | }

26 | const subs = [ | ^ 27 | hook.sub( 28 | 'renderer-attached', 29 | ({ View compiled (anonymous function) src/backend.js:179 176 | hook.emit('shutdown'); 177 | }); 178 | 179 | initBackend(hook, agent, window); | ^ 180 | 181 | // Setup React Native style editor if the environment supports it. 182 | if (resolveRNStyle != null || hook.resolveRNStyle != null) {` React version:

Steps To Reproduce

  1. create-react-app
  2. yarn add react-devtools
  3. in index.js add import 'react-devtools' on first line
  4. start react-devtools
  5. start create-react-app devserver
  6. go to localhost:3000
  7. crash

Link to code example:

add react-devtools on first line of vanilla 'create-react-app'

The current behavior

devtools

The expected behavior

Debug-Session

bvaughn commented 3 years ago

The "hook" defined on window.__REACT_DEVTOOLS_GLOBAL_HOOK__ does not have any of the subscription methods (including sub) which causes the code to error. I think this is because the hook is actually being initialized by Fast Refresh: https://github.com/facebook/react/blob/b51a686a93b6670fc80ae21d1a649194b3b723b2/packages/react-refresh/src/ReactFreshRuntime.js#L439-L466

Then the embedded DevTools backend is trying to use it.

I don't know that it makes sense to try to embed the DevTools backend via an import like this anymore anyway. The way I've been using when I want to inspect Safari is to load via a <script> tag like so:

<script src="http://localhost:8097"></script>

To my knowledge, the backend import path exists primarily to support React Native integration.

bvaughn commented 3 years ago

Don't suppose you know of any specific cases for import-in-the-browser, @gaearon?

Maybe it's easier to do conditionally, only in DEV?

bvaughn commented 3 years ago

I could fix DevTools by expanding the hooks check to verify that a pre-installed hook also defines a sub method: https://github.com/facebook/react/blob/b51a686a93b6670fc80ae21d1a649194b3b723b2/packages/react-devtools-shared/src/hook.js#L19-L22

But this would break Fast Refresh.

bvaughn commented 3 years ago

Alternately, preferably, the DevTools backend just needs to be injected before the Fast Refresh runtime to avoid the whole issue. (This is how the browser extension works for supported browsers like Chrome/Edge/Firefox.) I don't know how to guarantee that with Fast Refresh though, since it gets auto-injected before any user code is run.

Jack-Works commented 3 years ago

Having this problem too. e.sub is not a function

Jack-Works commented 3 years ago

I'm using react-devtools-core and it also broken

mecirmartin commented 3 years ago

Hey, i'm working on application which extensively uses embedded react-devtools-inline I'm currently trying to implement Fast Refresh, and i encountered this problem, i made sure the react-inline devtools are initialized beforehand. connectDevtoolsAction() just sends message through window.postMessage() to our frontend to initialize react-inline-devtools and initialization is completely synchronous

connectDevtoolsAction()
console.log(reactRefreshRuntime, window.__REACT_DEVTOOLS_GLOBAL_HOOK__)
reactRefreshRuntime.injectIntoGlobalHook(contentWindow)
// @ts-ignore
contentWindow.$RefreshReg$ = () => {}
// @ts-ignore
contentWindow.$RefreshSig$ = () => type => type

This throws hook.sub() is not a function as described in this issue. However when i add some artifical delay between initialization of devtools and call of `injectIntoGlobalHook(contentWindow), the issue goes away

 connectDevtoolsAction()
 await delay(1)
 console.log(reactRefreshRuntime, window.__REACT_DEVTOOLS_GLOBAL_HOOK__)
 reactRefreshRuntime.injectIntoGlobalHook(contentWindow)
 // @ts-ignore
 contentWindow.$RefreshReg$ = () => {}
 // @ts-ignore
 contentWindow.$RefreshSig$ = () => type => type

__REACT_DEVTOOLS_GLOBAL_HOOK__ is the same with/without the delay and it looks initialized to me

Screenshot_2021-04-23_at_06 48 16

@bvaughn any idea why this artificial delay is needed? Is initialization of devtools asynchronous in some way?

mecirmartin commented 3 years ago

UPDATE: Adding artificial delaay fixes the error, but breaks the connection of inline-devtools. So i am still unable to use react-devtools-inline with fast refresh

mecirmartin commented 3 years ago

Update: For anyone struggling with this, it turns out, you have to wait for specific event of react-devtools, then call reactRefreshRuntime.injectIntoGlobalHook(contentWindow) and it will work

window.addEventListener("message", (e) => {
  if (e.data.type === "React::DevTools::getSavedPreferences") {
    console.log("react devtools::getSavedPreferences");

    //@ts-ignore
    contentWindow.$RefreshRuntime$ = reactRefreshRuntime;
    //@ts-ignore
    reactRefreshRuntime.injectIntoGlobalHook(contentWindow);
    //@ts-ignore
    console.log("this", contentWindow.$RefreshRuntime$);
    // @ts-ignore
    contentWindow.$RefreshReg$ = () => {};
    // @ts-ignore
    contentWindow.$RefreshSig$ = () => (type) => type;
  }
});

Edited for formatting by @bvaughn.

feimosi commented 3 years ago

@mecirmartin you could have posted the code as a snippet to copy, that would help :slightly_smiling_face:

mecirmartin commented 3 years ago

@feimosi updated the comment

mecirmartin commented 3 years ago

@feimosi @Himself65 My solution works perfect in google chrome, but i just realized that it doesn't work in Safari. Did you implement fast-refresh with react-inline-devtools? Did you have any problems running in Safari? Firefox works great FYI

bvaughn commented 3 years ago

I advise against a fix that relies on listening to internal DevTools messages and inferring things about the timing. These messages, or the ordering of them, could change at any point in a minor update b'c they aren't considered part of the public API (so no semver).

feimosi commented 3 years ago

@mecirmartin I later encountered some other issues so I gave up on this. When I have more time I'll give it another try.

mecirmartin commented 3 years ago

@bvaughn So is there another way, to get fast refresh and inline-devtools working togetheer?

bvaughn commented 3 years ago

@mecirmartin I have not had a chance to dig into this issue, so I can't say. But I did want to caution against depending on a solution like the one mentioned above.

tjx666 commented 2 years ago

same here, developing chrome extension using standalone react-devtools.

image

tjx666 commented 2 years ago

reproduce:

  1. clone https://github.com/tjx666/awesome-chrome-extension-boilerplate.git
  2. checkout to develop branch
  3. pnpm install
  4. run pnpm devtools
  5. chrome open chrome://extensions/
  6. enable chrome extension develop mode
  7. load unpack extension folder under project root
  8. open options page of this extension
  9. then you will see the error in above screenshot.
yusufkhan07 commented 2 years ago

I am building a Zoom App which uses safari embedded browser. I am facing the issue mentioned in this post!

inspiraller commented 2 years ago

I have same issue using react-refresh. Thankyou - @mecirmartin I thought this solved my problem but only on refreshing page, not loading first time

My babel file:

module.exports = api => {
  const BABEL_ENV = api.env();
  const config = {
    presets: [
      "@babel/react",
      "@babel/typescript",
      ["@babel/env", { modules: false }],
    ]
  }
  if (BABEL_ENV === 'development') {
    config.plugins.unshift('react-refresh/babel');
  }
  return config
};

My index file

import "regenerator-runtime/runtime"; //or polyfill babel - needed for redux-saga
import React, { FC } from "react";
import ReactDOM from "react-dom";
import { Provider } from "react-redux";
import App, {Props as AppProps } from "src/Main/App";
import './reactRefreshFix' // @mecirmartin  - your code here... Fixes it
...

My webpack file:

...
const ReactRefreshPlugin = require("@pmmmwh/react-refresh-webpack-plugin");
const config = {
  entry: {
    reactRefreshSetup:
      "@pmmmwh/react-refresh-webpack-plugin/client/ReactRefreshEntry.js",
    app: path.resolve(__dirname, `./src/${startFile}`),
  },
  ...
   plugins: [
    new ReactRefreshPlugin(),
TimeBather commented 4 months ago

I have same issue when using react-refresh. And here is my solution(NOTICE: IT'S NOT OK FOR ALMOST EVERYONE, IT USES A VERY BAD TRICK to get the object of react-devtools-core and ALMOST NOT WORK in your enviorment, just for reference):

const HmrHook = window.__REACT_DEVTOOLS_GLOBAL_HOOK__;
globalThis.window = {};
require("react-devtools-core");
const devToolsHook = globalThis.window.__REACT_DEVTOOLS_GLOBAL_HOOK__;
globalThis.window = globalThis;
Object.defineProperty(window,'__REACT_DEVTOOLS_GLOBAL_HOOK__',{value:devToolsHook});
const fiberIdMap = new Map();
['inject','onScheduleFiberRoot','onCommitFiberRoot','onCommitFiberUnmount'].forEach((n)=>{
    if(devToolsHook[n]){
        let originalHook = devToolsHook[n];
        devToolsHook[n] = function(...args){
            let hmrResp = HmrHook[n].bind(devToolsHook)(...args);
            if( n === 'onScheduleFiberRoot' || n === 'onCommitFiberRoot'){
                args[0] = fiberIdMap.get(args[0]);
            }
            let originalResp = originalHook.bind(devToolsHook)(...args);
            if( n === 'inject' ){
                fiberIdMap.set(hmrResp,originalResp);
            }
            return hmrResp;
        }
    }else devToolsHook[n] = HmrHook[n];
})
HmrHook['renderers'].forEach = function(...args){
    return devToolsHook['renderers'].forEach(...args);
}