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

Warning for Async HOC component #1069

Closed
Zdend opened this issue Sep 17, 2018 · 8 comments
Closed

Warning for Async HOC component #1069

Zdend opened this issue Sep 17, 2018 · 8 comments

Comments

@Zdend
Copy link

Zdend commented Sep 17, 2018

Whenever I'm importing async modules with react-loadable that I later wrap with a HOC
I get a following warning:

[HMR] Checking for updates on the server...
11:50:00.895 console.js:35 [HMR] Updated modules:
11:50:00.895 console.js:35 [HMR]  - ./src/scripts/apps/enb/modules/api-connector/index.js
11:50:00.896 console.js:35 [HMR]  - undefined
11:50:00.896 console.js:35 [HMR]  - undefined
11:50:00.897 console.js:35 [HMR]  - undefined
11:50:00.897 console.js:35 [HMR] App is up to date.
11:50:00.945 console.js:35 React-hot-loader: a defaultProps(RegisterModuleWrapper) was found where a Component was expected.
          function DefaultProps(ownerProps) {
      return factory(ownerProps);
    }
console.(anonymous function) @ console.js:35
warn @ react-hot-loader.development.js:181
(anonymous) @ react-hot-loader.development.js:1443
hotReplacementRender @ react-hot-loader.development.js:1352
hotReplacementRender$1 @ react-hot-loader.development.js:1477
reconcileHotReplacement @ react-hot-loader.development.js:1486
renderReconciler @ react-hot-loader.development.js:1500
asyncReconciledRender @ react-hot-loader.development.js:1508
proxiedRender @ react-hot-loader.development.js:629
finishClassComponent @ react-dom.development.js:13393
updateClassComponent @ react-dom.development.js:13356
...

The reason is most likely the fact that I'm wrapping my async component with a HOC defaultProps on component load. If I don't do it (which I can't do it because I can only set defaultProps HOC during runtime), warning disappears. Warning also disappears if I wrap the whole module with hot(module)(MyAsyncComponent). The problem with the latter is that I have many components that would need to be wrapped like that and the whole HMR is working fine even without it, it just generates this warning that I would love to fix, ideally without suppressing warning logs.

// ... Async Component Module
export default {
    Component: connect(mapStateToProps)(MyComponent),
    Reducers
};
// ... Async component loader that register reducers on load and returns component
export const loadComponent: LoadComponentType = (ComponentLoader, props) =>
    Loadable({
        loader: () => ComponentLoader().then(Module => ReduxRegister.registerModule(Module.default, props)),
        loading: LoadingComponent
    });
// ... ReduxRegister
registerModule = (Module: Module, props: ?Object = {}): ?React.ComponentType<*> => {
    const { Component, Reducers } = Module;

    this.registerReducers(Reducers);
    return defaultProps(props)(Component);
};

I'm running the latest version of everything (webpack 4, babel 7, react 16.5, RHL 4.3.8)

Thanks!

@theKashey
Copy link
Collaborator

Quite interesting approach. It would be great if you will provide any example to play with.
Optionally - could you show the code for defaultProps?

I am almost absolutely sure about two things:

  1. There is at least one error inside RHL react tree traversal
  2. It's 99.9% error proof.

Probably here problem is with components displayName - it could be different for rendered and not-yet rendered components - you may set a break point in the place where we are comparing old and new type and double check "how" they are different in real.

@Zdend
Copy link
Author

Zdend commented Sep 17, 2018

It might take me some time to assemble a reproducible repo. However I set a breakpoint where the warning is and these are the values - not sure whether it will be useful at all.

Code for defaultProps can be found here https://github.com/acdlite/recompose/blob/master/src/packages/recompose/defaultProps.js

screen shot 2018-09-17 at 6 04 17 pm

@theKashey
Copy link
Collaborator

Ok, look like the problem is about seeing "LoadingComponent" in React tree, while a "target" component has been rendered by fact. Somehow similar to #1050

This happens cos RHL is "always enabled" and trying to did it's job even without actual "HMR" event.

Right now there is no way you can solve this issue (this is not a real issue) - probably just suppress warning. Everything should work - it just reporting about situation it could not handle, and shall not handle.

@Zdend
Copy link
Author

Zdend commented Sep 17, 2018

Thanks for taking the time! I'll most likely try to wrap every async component in hot(module) as it clears out undefined in the Updated modules list and solves the warning, it's just a little bit cumbersome to do it for every async component.

11:50:00.895 console.js:35 [HMR] Updated modules:
11:50:00.895 console.js:35 [HMR]  - ./src/scripts/apps/enb/modules/api-connector/index.js
11:50:00.896 console.js:35 [HMR]  - undefined
11:50:00.896 console.js:35 [HMR]  - undefined
11:50:00.897 console.js:35 [HMR]  - undefined

@theKashey
Copy link
Collaborator

  1. Please dont wrap anything with hot without special need
  2. disable logging (actually look like you enabled it)
 import {setConfig} from 'react-hot-loader';
setConfig({logLevel: 'error'}); // and this is default value!

@Zdend
Copy link
Author

Zdend commented Sep 17, 2018

The reason why I enabled logging in the first place was to find out why Updated modules are undefined as I was getting a pretty long list of undefined. I'm not sure whether it's related to the first issue with the warning but if I wrapped it with hot it actually solved the undefined logging so I thought that I must be doing something right :-)

@theKashey
Copy link
Collaborator

Try to find the origin of [HMR] - undefined and find why it is undefined. There is no way to create a module without id - something could be very broken

@Zdend
Copy link
Author

Zdend commented Sep 17, 2018

Thanks man, it seems to be related to webpack-hot-middleware - I'll continue my investigation over there. webpack-contrib/webpack-hot-middleware#306

@Zdend Zdend closed this as completed Sep 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants