Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

react-refresh load from CDN? #17552

Closed
otakustay opened this issue Dec 9, 2019 · 12 comments
Closed

react-refresh load from CDN? #17552

otakustay opened this issue Dec 9, 2019 · 12 comments

Comments

@otakustay
Copy link
Contributor

otakustay commented Dec 9, 2019

Do you want to request a feature or report a bug?

feature

What is the current behavior?

When react-dom is loaded from CDN like <script src="https://cdn.jsdelivr.net/npm/react-dom@16.12.0/umd/react-dom.development.js"></script>, react-refresh failed to inject hook into devtools.

I've created a related issue here: pmmmwh/react-refresh-webpack-plugin#13

We should find a way to invoke injectIntoGlobalHook function from react-refresh/runtime, however this file is in cjs format so we cannot currently do this in a simple way.

What is the expected behavior?

I'd like react-refresh to publish runtime as a umd bundle so we can reference it from CDN and put it before react-dom's <script> element, then invoke injectIntoGlobalHook in the right place.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

  • react-dom: 16.12.0
  • react-refresh: 0.7.0
@otakustay
Copy link
Contributor Author

Is it possible that react-dom handle injection lazy on the first render call? This way externals tools can hook injections more easily

@gaearon
Copy link
Collaborator

gaearon commented Dec 17, 2019

Can you check if #17633 fixes it?

@gaearon
Copy link
Collaborator

gaearon commented Dec 17, 2019

I verified #17633 fixes this.

@gaearon
Copy link
Collaborator

gaearon commented Dec 17, 2019

Fixed in react-refresh@0.7.1.

@thetrime
Copy link

I realise this issue is already over a year old, but I'm not sure if this is really fixed. If you load react-dom first, then injectInternals() will return immediately if REACT_DEVTOOLS_GLOBAL_HOOK is not defined. If we then load react-refresh, it will check if REACT_DEVTOOLS_GLOBAL_HOOK is defined, and if not, define it by itself, but there doesn't seem to be any attempt to re-inject the hooks. The react-dom code doesn't set injectedHook except in injectInternals(), and if injectedHook is null then onCommitRoot() will have no effect.

The upshot is that loading react-dom as an external from a webpack project means it'll always load before react-refresh if that's bundled in (and it seemingly has to be, since I don't see it on a CDN).

Should I open a new issue for this, or am I missing something obvious? FWIW, it works fine if I have the devtools installed. If I uninstall them, I get no HMR (or rather, I get all the normal activity for HMR but no actual changes to the DOM). If I include react-dom inside my bundle so it loads after react-refresh, then everything works in all cases.
I'm using react-17.0.1 (though I also tested with 16.10) and react-refresh-0.9.0

@pmmmwh
Copy link

pmmmwh commented Feb 13, 2021

Should I open a new issue for this, or am I missing something obvious? FWIW, it works fine if I have the devtools installed. If I uninstall them, I get no HMR (or rather, I get all the normal activity for HMR but no actual changes to the DOM). If I include react-dom inside my bundle so it loads after react-refresh, then everything works in all cases.
I'm using react-17.0.1 (though I also tested with 16.10) and react-refresh-0.9.0

Can you provide a bit more details on how you're setting up your project, how are you externalising React and what versions (including dev vs prod build) of React/ReactDOM/ReactRefresh you're using?

@thetrime
Copy link

thetrime commented Feb 15, 2021

Can you provide a bit more details on how you're setting up your project, how are you externalising React and what versions (including dev vs prod build) of React/ReactDOM/ReactRefresh you're using?

I can do one better than that - here's a simple(as possible) self-contained example: https://github.com/thetrime/hmr-test

You should just be able to npm install && npm start. Then try editing src/App.jsx to change the content of the page and save it. I see all the updates in the network tab in my browser, but no actual changes are reflected unless I reload.

The README.md contains a description of what the problem appears to be at a code level. I'm using dev builds of all 3 modules, versions 17.0.1 for React/ReactDOM and 0.9.0 for ReactRefresh. The repository illustrates how I'm externalising it, but the summary is that in webpack.config.js I have

    externals: {
        "react": "React",
        "react-dom": "ReactDOM"
    },

and in my index.html I include CDN links for both of these in the head section. I experimented with trying to get the bundle to load before the CDN links, but that seems like a risky approach to take (and I couldn't get it to work anyway).

Hopefully I'm doing something foolish, but looking at the code for react-dom and react-refresh I can't see how this could possibly work if react-dom is ever loaded first, and my understanding of this issue was that it was supposed to address that specific case.

@nordfjord
Copy link

We're having the same issue as thetrime.

We're using the development builds of react and react-dom and dynamically loading applications via SystemJS.

This means react-dom always loads before react-refresh in our scenario.

A workaround for now seems to be to install the react-devtools chrome extension as that makes the __REACT_DEVTOOLS_GLOBAL_HOOK__ available

@pmmmwh
Copy link

pmmmwh commented Jul 20, 2021

As of now, there is no reliable way to make React Refresh work without having the ReactRefreshRuntime.injectIntoGlobalHook call execute before any React renderer code is initialised.

Either you would have to internalise React during development, install and use the react-devtools extension, or ensure that the injection order is correct.

However - given how much this have been brought up in bug reports we might provide a tiny script where you can run from any package delivery CDN to inject a stub of the global hook in the next version of the Webpack plugin.

@dqm07
Copy link

dqm07 commented Aug 26, 2022

@pmmmwh is tiny script out? in my project i need to use it

@Hexi1997
Copy link

Hexi1997 commented Apr 5, 2023

@pmmmwh is tiny script out? in my project i need to use it

i need too

@wwglala
Copy link

wwglala commented Feb 5, 2024

@pmmmwh i need too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants