Skip to content

[guide] [react] add note about deprecated lifecycles #1993

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

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

Conversation

meetzaveri
Copy link

There were still deprecated lifecycle being mentioned in two places. Thus by removing them, it will make clear sense of latest React API after 16.2.

Copy link
Collaborator

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

These are not actually "deprecated" in the sense that they'll only be "unsafe with async rendering". Either way, we should leave these here, and let React itself (and eslint-plugin-react rules) warn on these methods as needed.

@meetzaveri
Copy link
Author

meetzaveri commented Jan 16, 2019

These are not actually "deprecated" in the sense that they'll only be "unsafe with async rendering".

This makes sense. But can we like address developer that this lifecycles are now no more in use like by adding a label in parenthesis or making seperate section for it ?

@ljharb
Copy link
Collaborator

ljharb commented Jan 16, 2019

I think a section for it that calls out that only in React 16.3+, certain methods should be avoided in favor of others, would be useful, yes.

@meetzaveri
Copy link
Author

Okay. So should I make changes now in readme file acc. to it?

@ljharb
Copy link
Collaborator

ljharb commented Jan 17, 2019

Updating this PR to do that would be great!

@meetzaveri
Copy link
Author

Sure. So should I finally add a section about avoiding lifecycles?

@ljharb
Copy link
Collaborator

ljharb commented Jan 18, 2019

Not avoiding; but using the ones appropriate for your React version. Not everyone is on the latest version.

@@ -701,6 +702,15 @@ We don’t recommend using indexes for keys if the order of items may change.
> Why? [`isMounted` is an anti-pattern][anti-pattern], is not available when using ES6 classes, and is on its way to being officially deprecated.

[anti-pattern]: https://facebook.github.io/react/blog/2015/12/16/ismounted-antipattern.html

## Things to avoid
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't the right phrasing here; someone using React 16.2 shouldn't be avoiding anything.


## Things to avoid

Using deprecated APIs will create bad legacy code. Hence it's good to know that which APIs have been deprecated.
Copy link
Collaborator

Choose a reason for hiding this comment

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

All of these APIs are not "bad" or "legacy", they're solely not safe to use with async rendering. Can you rephrase this to have less of a judgemental connotation?


Using deprecated APIs will create bad legacy code. Hence it's good to know that which APIs have been deprecated.

These Component lifecycles are been deprecated since React version 16.3+ :
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we add, for each one, which methods should be used instead (with links to their documentation)?

@ljharb ljharb changed the title [FIX] : Removed deprecated lifecycles [guide] [react] add note about deprecated lifecycles Jan 19, 2019
@MdAli2k

This comment has been minimized.

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

Successfully merging this pull request may close these issues.

3 participants