-
Notifications
You must be signed in to change notification settings - Fork 782
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
base: master
Are you sure you want to change the base?
Conversation
Hello, this is a fairly important update. Anything we can do to help get some action on this? |
Hello @rscotten ,
Just merge it, please... Thanks |
Without this PR, the library currently spews warnings from React on every server restart and every browser
console session.
A very simple & straightforward merge, so it would be greatly appreciated.
…On Tue, 5 Nov 2019, 21:14 SMRSAN, ***@***.***> wrote:
Hello @rscotten <https://github.com/rscotten> ,
Anything we can do to help to get some action on this?
Just merge it, please...
Thanks
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#213?email_source=notifications&email_token=AAN7XTQW7QQV4W6KYRDXMHDQSHO3PA5CNFSM4JATXPYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDEK5ZA#issuecomment-550022884>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAN7XTWRPSFXG2FEBV7JQX3QSHO3PANCNFSM4JATXPYA>
.
|
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..! |
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. |
Uh oh... |
I've just tried both |
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. |
@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. |
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.
Please update the following components: LoadableComponent Is there any fixed date for this pull request to be merged? |
Hey @rscotten , I think @ramirost is right as he said:
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. 🙏 ) |
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) |
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? |
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. |
@iKonrad Oh snap, yeah, good call. I'm bailing on this library and moving over to |
@rscotten is it the same API? |
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 |
Don't bother to beg the author. Let use this: https://github.com/gregberge/loadable-components |
I bother because |
React infosFor those interested:
SolutionWe 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 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. AliasesIf you don't want to rename all your imports to
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 :) |
FYI, This should solve this other warning:
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"
}, |
这是来自QQ邮箱的假期自动回复邮件。你好,我最近正在休假中,无法亲自回复你的邮件。我将在假期结束后,尽快给你回复。
|
Hello dear maintainers,
Please merge and publish this new code to omit the warning message. 🙏
Thanks