facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
227.1k stars 46.3k forks source link

calculating context changes in componentDidUpdate #14398

Open Lexicality opened 5 years ago

Lexicality commented 5 years ago

Do you want to request a feature or report a bug? Bug

What is the current behavior? There is no way to see if componentDidMount was caused by a context change

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:

import React from 'react';

interface ExampleContext {
    param: string;
}

const ExampleContext = React.createContext<ExampleContext>({
    param: 'foo',
});

interface ExampleState {
    data: any;
}

class Example extends React.Component<{}, ExampleState> {
    static contextType = ExampleContext;
    context!: ExampleContext;

    constructor(props) {
        super(props);

        this.state = {
            data: [],
        };
    }

    fetchData(param: string): void {
        // whatever
    }

    componentDidMount() {
        this.fetchData(this.context.param);
    }

    componentDidUpdate() {
        // ???        
    }
}

What is the expected behavior? A 4th parameter for previousContext

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React? 16.6

nanmon commented 5 years ago

You can save the previous context value in the component state

Lexicality commented 5 years ago

Could you give an example how? I would have thought that without #13970 it would be the same problem

mgenev commented 5 years ago

this is a big problem i'm running into, it causes an infinite loop that I have no good way of preventing it

JoshClose commented 4 years ago

@mgenev You can set an instance variable for previousContext in componentDidMount then check it in componentDidUpdate against this.context. You'll need to update the previous context here also. This is still just a workaround. I'm annoyed that context is type any also.

class MyComponent extends Component {
    private previousContext;

    static contextType = MyContext;

    componentDidMount() {
        this.previousContext = context;
    }

    componentDidUpdate() {
        if (this.previousContext.someProperty !== this.context.someProperty) {
            this.doSomething();
        }

        this.previousContext = this.context;
    }
}
stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

Lexicality commented 4 years ago

This issue is also still relevant, unless class components are being deprecated?

JoshClose commented 4 years ago

I sure hope not. I went full in on hooks and found I really don't like them. Class components with typescript are so much better IMO.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

JoshClose commented 4 years ago

Bump.

Lexicality commented 4 years ago

I would appreciate input from the React team on whether or not class components are deprecated

tennox commented 4 years ago

@Lexicality Read the docs: https://reactjs.org/docs/hooks-intro.html#gradual-adoption-strategy

TLDR: There are no plans to remove classes from React.

Lexicality commented 4 years ago

Removing and deprecation aren't the same thing. I'd like to know if I need to keep bothering to bump these real issues regarding context and class components or if I should let the stalebot kill them all because class components are dead and just being allowed to float behind functional components + hooks.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

JoshClose commented 4 years ago

bump

slavik-chapelskyi commented 3 years ago

@Lexicality As the workaround you can create HOC, pass context value as props and compare it in componentDidUpdate.

const withContextValue = Component => props => {
  return (
    <ExampleContext.Consumer>
      {value => <Component {...props} contextValue={value} />}
    </ExampleContext.Consumer>
  );
};

withContextValue(
  class DemoComponent extends Component {
    componentDidUpdate(prevProps) {
      if (prevProps.contextValue !== this.props.contextValue) {
        //... do something
      }
    }
  }
);
bvaughn commented 3 years ago

I would appreciate input from the React team on whether or not class components are deprecated

We don't have plans to remove class components. We also don't have plans to expand the class component API.

Lexicality commented 3 years ago

Does that mean this issue and #13970 won't be fixed? Both of them caused fairly serious issues for me using the new context API in the past. I had to resort to workarounds like the one @slavik-chapelskyi suggested which make the contextType attribute basically useless.