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

Hot reloading does not work with @babel/plugin-transform-modules-commonjs #120

Closed
michaelgmiller1 opened this issue Jun 18, 2020 · 11 comments · Fixed by #129 or #140
Closed

Hot reloading does not work with @babel/plugin-transform-modules-commonjs #120

michaelgmiller1 opened this issue Jun 18, 2020 · 11 comments · Fixed by #129 or #140
Labels
help wanted Extra attention is needed

Comments

@michaelgmiller1
Copy link

michaelgmiller1 commented Jun 18, 2020

When I use the @babel/plugin-transform-modules-commonjs plugin and change the repo to use an object export for App, hot reloading breaks with the following error:

[HMR] Cannot apply update. Need to do a full reload!
VM1440 react_devtools_backend.js:6 [HMR] Error: Aborted because ./src/App.jsx is not accepted
Update propagation: ./src/App.jsx -> ./src/index.js -> 1
    at hotApplyInternal (http://localhost:8080/main.js:537:30)
    at hotApplyInternal (http://localhost:8080/main.js:770:20)
    at hotApply (http://localhost:8080/main.js:391:19)
    at http://localhost:8080/main.js:366:22

I've developed a minimum repro here: https://github.com/michaelgmiller1/react-refresh-webpack-plugin/tree/transform-repro

After patching, you can run the webpack-dev-server example and modify the App.jsx file to reproduce.

Let me know if you need any more info!

Thanks!
Mike

@michaelgmiller1 michaelgmiller1 changed the title Hot reloading does not work with @babel/plugin-transform-runtime Hot reloading does not work with @babel/plugin-transform-modules-commonjs Jun 18, 2020
@michaelgmiller1
Copy link
Author

anything I can do to help investigate this? super excited to adopt react refresh in our app!

@brandonpapworth
Copy link

👍

@pmmmwh
Copy link
Owner

pmmmwh commented Jun 25, 2020

I haven't had a lot of time to look at this just yet, but having the transformed source of ./App.jsx will help diagnose the issue (most HMR bailouts are caused by exports failed to be detected as "React-related-only").

@michaelgmiller1
Copy link
Author

I reduced App.jsx to a minimum to reproduce:

import * as React from 'react';
import ClassDefault from './ClassDefault';

export function App() {
  return (
    <div>
      This is App.jsx!!!!!!
    </div>
  );
}

The result is:

/* 11 */
/***/ (function(module, exports, __webpack_require__) {

"use strict";


var _interopRequireDefault = __webpack_require__(4);

var _interopRequireWildcard = __webpack_require__(1);

Object.defineProperty(exports, "__esModule", {
  value: true
});
exports.App = App;

var React = _interopRequireWildcard(__webpack_require__(0));

var _ClassDefault = _interopRequireDefault(__webpack_require__(12));

function App() {
  return /*#__PURE__*/React.createElement("div", null, "This is App.jsx!!!!!!");
}

_c = App;

var _c;

$RefreshReg$(_c, "App");

/***/ }),

@michaelgmiller1
Copy link
Author

michaelgmiller1 commented Jun 25, 2020

So it looks like it does:

Object.defineProperty(exports, "__esModule", {
  value: true
});
exports.App = App;

on the app component. Should that interfere with hot reloading?

@pmmmwh
Copy link
Owner

pmmmwh commented Jun 26, 2020

I did a bit of digging and it seems that this only happens when @babel/plugin-transform-modules-commonjs and @babel/plugin-transform-runtime is both used.

The culprit of this issue is that somehow the referential equality of the object which we use to calculate whether the change is acceptable is broken - I'll have to look further (and maybe even ping Dan) to get this properly fixed.

@pmmmwh
Copy link
Owner

pmmmwh commented Jun 26, 2020

@michaelgmiller1 I think I've found the culprit.

The issue in the repro is that there are two instances of react-refresh/runtime being used, and thus object referential equality will be broken across the two instances. I haven't figured out why the combination of the two plugins will cause this though - it might have something to do with helper injection. (Also unsure why the issue didn't surface earlier ...)

After making sure that only one copy is loaded, I seem to have successful results.
See CodeSandbox example here

@michaelgmiller1
Copy link
Author

@pmmmwh thanks for diving in! just one comment on your codebox, it uses export default App instead of export function App. Exporting via object was needed to reproduce the issue for me. If I change your code sample to export via object, then it breaks again.

@pmmmwh
Copy link
Owner

pmmmwh commented Jun 26, 2020

@pmmmwh thanks for diving in! just one comment on your codebox, it uses export default App instead of export function App. Exporting via object was needed to reproduce the issue for me. If I change your code sample to export via object, then it breaks again.

This ... is ... so ... strange!

I've tested with a bare bones example on my local machine with export function App and it works 😱

Edit: Okay - so it seems that if the file have imports with classes/default exports it will break HMR. Investigating further.

@pmmmwh pmmmwh added the help wanted Extra attention is needed label Jun 26, 2020
@pmmmwh
Copy link
Owner

pmmmwh commented Jun 26, 2020

Uhh ... okay. I have something working. I will add a regression test and some notes in the code base.

@pmmmwh
Copy link
Owner

pmmmwh commented Jul 12, 2020

RE-FIXED in 0.4.0-beta.8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
3 participants