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

Added UNSAFE_ prefix to componentWillMount() #213

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

smrsan76
Copy link

Hello dear maintainers,

Please merge and publish this new code to omit the warning message. 🙏

Thanks

@rscotten
Copy link

rscotten commented Nov 1, 2019

Hello, this is a fairly important update. Anything we can do to help get some action on this?

@smrsan76
Copy link
Author

smrsan76 commented Nov 5, 2019

Hello @rscotten ,

Anything we can do to help to get some action on this?

Just merge it, please...

Thanks

@muntasirsyed
Copy link

muntasirsyed commented Nov 6, 2019 via email

@smrsan76
Copy link
Author

smrsan76 commented Nov 8, 2019

Hi @muntasirsyed ,

Thanks for paying attention to this PR.

As you could see here, I've just edited one line of code.

I really don't have any idea about where the error message comes out from..!
Because my PR is just related to editing one line of code in React, not test codes.

@ramirost
Copy link

ramirost commented Nov 8, 2019

Hi @muntasirsyed ,

Thanks for paying attention to this PR.

As you could see here, I've just edited one line of code.

I really don't have any idea about where the error message comes out from..!
Because my PR is just related to editing one line of code in React, not test codes.

The error raises because UNSAFE_ was added in react 16.3 and the project depends on react 16.0, a version bump on react is needed to fix this.

@rscotten
Copy link

rscotten commented Nov 8, 2019

@smrsan76 Can you try @ramirost 's advice and bump the react dependency from 16.0.0 to 16.3.0 in package.json?

@smrsan76
Copy link
Author

smrsan76 commented Nov 9, 2019

Uh oh...
It seems that v16.11.0 (currently, the latest version) isn't compatible with this package.
Let me try v16.3.0 as @ramirost said...

@smrsan76
Copy link
Author

smrsan76 commented Nov 9, 2019

I've just tried both v16.11.0 (the latest) and v16.3.0, but none of them fixed the problem...

@ramirost
Copy link

ramirost commented Nov 10, 2019

I have the code with the minimum version upgrade on react components (to 16.3.x) in this branch: https://github.com/ramirost/react-loadable/tree/unsafe_prefix with the tests passing.

Should I create a new PR or maybe @smrsan76 can uptate this PR?

I attach also the patch file over master of the original repo of the unique commit on that branch that could fix this warning.

patch.diff.txt

@rscotten
Copy link

@ramirost Please create a new PR and then post a comment here linking to the new PR. If it passes and merges, we can close this PR

@ramirost
Copy link

@ramirost Please create a new PR and then post a comment here linking to the new PR. If it passes and merges, we can close this PR

I created PR #217 but tests already failed, then looking at previous tests results I see that the PR #216 solves the same issue and also solves the build problem (which was only on node 4 build for my PR), maybe we just need to merge PR #216 which seems OK and reject both this one and the one I just created.

@empav
Copy link

empav commented Nov 11, 2019

VM425 backend.js:6 Warning: componentWillMount has been renamed, and is not recommended for use. See https://fb.me/react-async-component-lifecycle-hooks for details.

  • Move code with side effects to componentDidMount, and set initial state in the constructor.
  • Rename componentWillMount to UNSAFE_componentWillMount to suppress this warning in non-strict mode. In React 17.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, you can run npx react-codemod rename-unsafe-lifecycles in your project source folder.

Please update the following components: LoadableComponent

Is there any fixed date for this pull request to be merged?

@smrsan76
Copy link
Author

Hey @rscotten , I think @ramirost is right as he said:

I created PR #217 but tests already failed, then looking at previous tests results I see that the PR #216 solves the same issue and also solves the build problem (which was only on node 4 build for my PR), maybe we just need to merge PR #216 which seems OK and reject both this one and the one I just created.

So, please take a look at PR #216 and merge it, if it hasn't any other issues...

( I also have to apologize for delays between my contributions. 🙏 )

@iKonrad
Copy link

iKonrad commented Nov 30, 2019

Yes it'd be good to update the codebase for the recent React changes. @jamiebuilds any chance to have this fixed? (I can jump in and help if the library is going to be maintained)

@kopax
Copy link

kopax commented Dec 6, 2019

Hello sorry if it's already in progress. I use this awesome package a lot (for each routes of my application) and thus produce an infinite number of warning in all our developer's console. Could you please merge the proposed solution or let us know if this won't be solved?

@iKonrad
Copy link

iKonrad commented Dec 6, 2019

Alright, it seems that this library is not going to get support anytime soon. According to the official documentation, I advise you to move over to the loadable-components library as it is getting much more attention. I've done the migration in one project and it's pretty straightforward.

@rscotten
Copy link

rscotten commented Dec 6, 2019

@iKonrad Oh snap, yeah, good call. I'm bailing on this library and moving over to @loadable/component

@kopax
Copy link

kopax commented Dec 7, 2019

@rscotten is it the same API?

@kopax
Copy link

kopax commented Dec 16, 2019

For thoose who want to stick to the API and feature of this project, I have published on npm the fix: https://www.npmjs.com/package/@yeutech-lab/react-loadable

GitHub: https://github.com/yeutech-lab/react-loadable

@yumindev
Copy link

yumindev commented Dec 23, 2020

Don't bother to beg the author. Let use this: https://github.com/gregberge/loadable-components

MicrosoftTeams-image

@kopax
Copy link

kopax commented Dec 26, 2020

Don't bother to beg the author. Let use this: https://github.com/gregberge/loadable-components

MicrosoftTeams-image

I bother because React.lazy is poor, for example it does not have a preload method that can be triggered whenever you want (on <a> mouseover for example.)

@slorber
Copy link

slorber commented Feb 12, 2021

React infos

For those interested:

Solution

We use React-loadable in facebook/docusaurus. Until we migrate to loadable-components, I was looking for a solution to get rid of the warning.

I published a fork "@docusaurus/react-loadable": "5.5.0", with the minimal amount of changes to get rid of the warning (changes).

I didn't want to use someone else fork for security reasons, as our open-source project is widely used by thousands of users, as it would require me to inspect the published JS. If you use a fork, you'd rather fix the dependency to a specific version instead of using a semver range. In our case, it's very unlikely that we'll ever update this fork again.

Aliases

If you don't want to rename all your imports to @docusaurus/react-loadable, you can:

  • use a webpack alias in your Webpack config:
module.exports = {
  resolve: {
    alias: {
      "react-loadable": "@docusaurus/react-loadable"
    }
  }
};
import moduleAlias from "module-alias";

moduleAlias.addAliases({
  "react-loadable": "@docusaurus/react-loadable"
});

Hope anyone will find this useful :)

Repository owner deleted a comment from ygs-code Sep 6, 2022
@slorber
Copy link

slorber commented Apr 26, 2024

FYI, @docusaurus/react-loadable v6.0 is out to support React v18.3 (and likely React v19 too).

This should solve this other warning:

Warning: LoadableComponent uses the legacy contextTypes API which is no longer supported and will be removed in the next major release. Use React.createContext() with static contextType instead.
Learn more about this warning here: reactjs.org/link/legacy-context

Fork repo: https://github.com/slorber/react-loadable

I migrated to the "new" React context, and also removed the useless prop-type dependency.

This is provided as is, I don't plan to merge PRs to the fork, just solving my own needs. Hope you'll find it useful.

This is also a convenient way to use this package:

  "dependencies": {
    "react-loadable": "npm:@docusaurus/react-loadable@6.0.0"
  },

@fangyinghua
Copy link

fangyinghua commented Apr 26, 2024 via email

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