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

Fixed Unsafe warnings and some improvements. #222

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AlaguSelvan
Copy link

  1. node.js config updated in Travis CI/CD.
  2. Added node.js 12 & Removed node.js 4 support in Travis CI/CD.
  3. Replaced ComponentWillMount to ComponentDidMount.
  4. Removed Trailing Commas and Unused Imports.

@AlaguSelvan
Copy link
Author

@jamiebuilds @tajo this is a highly needed feature, can we get this merged?

@@ -158,7 +158,7 @@ function createLoadableComponent(loadFn, options) {
return init();
}

componentWillMount() {
componentDidMount() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could introduce bugs, why don't you go the safe route and use UNSAFE_componentWillMount for now?

If you really want to go directly to didMount, this._mounted = true would need to move it to the constructor.
Calling retry from the loading component before the mount phase could cause unexpected behaviors with this change, for example.
The _loadModule is probably fine to stay here.

Unfortunately sounds like nobody will ever merge this anyway...

@ensemblebd
Copy link

is the repo owner awol? I don't understand how a perfectly good pull request isn't acted upon.

sheesh. guess that's the nature of npm and all these insane changesets from insane vendors. One must clone their own, hardcode their changes, and hardlock their application to a specific version for a specific purpose, and just give up on the future.

@ghost
Copy link

ghost commented Mar 3, 2020

If there is no response from owner then we can manage an active fork where we can merge all these PR's. Any volunteers?

@reanimatedmanx
Copy link

Will this be merged any time soon? Or this repo is unmaintaned at this point with no chances of getting any active PR's merged?

@ghost
Copy link

ghost commented Mar 18, 2020

Now that react comes loaded with lazy, this library doesn't hold much value. You can implement the same functionality with React lazy.

Repository owner deleted a comment from ygs-code Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants