-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
react/no-unused-state
warning in new getDerivedStateFromProps
methods
#1759
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
Comments
Having the same issue here |
This may be resolved with the next release; I'll leave it open to check. |
Same issue here. |
Does not seem to be fixed in 7.8.2 |
@evan-scott-zocdoc can you file a new issue? |
@ljharb I could, but given the issue in this ticket isn't actually fixed and it's recent, shouldn't this just be reopened? |
@evan-scott-zocdoc sure. could you provide the exact code and warnings you get? |
Sure thing, here's the error:
and here's a code sample (I tried to clean it up a bit): export default function addInsuranceModal(WrappedComponent) {
class AddInsuranceModal extends React.Component {
static displayName = `AddInsuranceModal(${WrappedComponent.name || WrappedComponent.displayName || 'Component'})`;
static WrappedComponent = WrappedComponent;
static propTypes = {
bookingSpotlightIsOn: PropTypes.bool,
insuranceIsAccepted: PropTypes.bool,
locationId: PropTypes.string,
providerId: PropTypes.string.isRequired,
providerName: PropTypes.string,
};
static getDerivedStateFromProps(props, state) {
if (!props.insuranceIsValidating) {
const insuranceIsNoLongerAccepted = (
state.insuranceWasAccepted &&
props.insuranceIsAccepted === false
);
const insuranceJustChangedAndIsNotAccepted = (
state.insuranceJustChanged &&
props.insuranceIsAccepted === false
);
const insuranceNotAcceptedOnInitialLoad = (
props.insuranceIsAccepted === false &&
state.initialInsuranceLoad
);
const newState = { ...state };
const insuranceNotAccepted = (
insuranceIsNoLongerAccepted ||
insuranceJustChangedAndIsNotAccepted ||
insuranceNotAcceptedOnInitialLoad
);
/**
* on desktop, show modal on page load- on mobile, wait until
* user clicks book appointment button
*/
const displayReadyForModal = (
getWindowBreakpoint() > Breakpoints.medium ||
props.bookingSpotlightIsOn
);
if (insuranceNotAccepted && displayReadyForModal) {
newState.showModal = true;
newState.insuranceWasAccepted = props.insuranceIsAccepted;
newState.insuranceJustChanged = false;
newState.initialInsuranceLoad = false;
} else {
newState.insuranceWasAccepted = props.insuranceIsAccepted;
newState.insuranceJustChanged = false;
}
return newState;
}
return null;
}
state = {
initialInsuranceLoad: true,
insuranceJustChanged: false,
insuranceWasAccepted: false,
showModal: false,
};
mounted = false;
componentDidMount() {
this.mounted = true;
}
componentWillUnmount() {
this.mounted = false;
}
render() {
return (
<div>
{this.state.showModal && this.renderModal()}
<WrappedComponent
{...this.props}
onChange={this.onChangeInsurance}
/>
</div>
);
}
renderModal() {
const { providerName } = this.props;
return (
<Portal>
<ModalView />
</Portal>
);
}
onChangeInsurance = insurance => {
if (this.mounted) {
this.setState({
insuranceJustChanged: true,
});
}
this.props.onChange(insurance);
};
onClickBookAnyway = () => {
this.props.onClickBookAnyway();
this.onClickCancel();
};
onClickChangeInsurance = () => {
this.props.onClickChangeInsurance();
this.onClickCancel();
};
onClickCancel = () => {
this.setState({ showModal: false });
}
}
return withInsuranceValidation(AddInsuranceModal);
} Basically it's a typical HOC pattern of wrapping another component. |
Thanks, looks like it's def still an issue. |
Actually the rule did not detected the usage since you did not used the standard name for the I don't know if this is fixable without getting a lot of false positive. |
Hmm, not sure how I feel about that. Those variable names are a suggestion, not part of the API. |
I think if the method name is |
Just wondering where this 'standard' is defined ? It seems the React docs use
https://reactjs.org/docs/react-component.html#static-getderivedstatefromprops |
@bengeorge because those were the argument names on It doesn't really matter tho, you shouldn't have to name the arguments anything special. |
I also have this issue after refactoring componentWillReceiveProps into getDerivedStateFromProps, would be nice to have a fix for this |
for anyone else getting the same error and ends up here. i recently updated
so i guess to fix this error you can either update |
@Adnan-Bacic the airbnb config uses ^, so it should already give you the latest if you’re following the instructions correctly. The Airbnb config never needs to update anything unless it’s a semver-major update, nor will it publish solely for that. |
@ljharb according to the docs we have to run:
which shows:
which arent the latest for any of them. though i guess the ^ character means we can update to higher versions regardless. |
@Adnan-Bacic yes, it means you can, and it means npm always will, so you’ll always get the latest when you update. |
eslint versions
If the only place you reference state is inside
getDerivedStateFromProps
then the eslint warning triggers forno-unused-state
. See the example below:The text was updated successfully, but these errors were encountered: