Skip to content

Commit

Permalink
Fix StrictMode errors in FixedDataTableContainer (#740) (Fixes #738)
Browse files Browse the repository at this point in the history
When StrictMode is enabled, and FDT is run under React v19, we run into NPEs in the top-level `<FixedDataTableContainer>` component, causing the table to not function correctly.

StrictMode runs the constructor and lifecycle methods twice.
The order I observed was as follows:

```
constructor
constructor
getDerivedStateFromProps
getDerivedStateFromProps
componentDidMount
componentWillUnmount
componentDidMount
getDerivedStateFromProps
getDerivedStateFromProps
```

So when running in StrictMode, the handlers/redux store gets initialized in the constructor, and then immediately gets destroyed in componentWillUnmount, leading the table to be in a destroyed state.

I'm fixing this by moving the initialization logic to both the constructor and componentDidMount, following what we did in #657 in the past.
  • Loading branch information
pradeepnschrodinger authored Dec 16, 2024
1 parent d2e537b commit 3924dff
Showing 1 changed file with 45 additions and 17 deletions.
62 changes: 45 additions & 17 deletions src/FixedDataTableContainer.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,7 @@ class FixedDataTableContainer extends React.Component {
constructor(props) {
super(props);

this.reduxStore = FixedDataTableStore.get();

this.scrollActions = getScrollActions(this.reduxStore, () => this.props);

this.reduxStore.dispatch(initialize(props));

this.unsubscribe = this.reduxStore.subscribe(this.onStoreUpdate.bind(this));
this.state = {
boundState: FixedDataTableContainer.getBoundState(this.reduxStore), // the state from the redux store
reduxStore: this.reduxStore, // put store instance in local state so that getDerivedStateFromProps can access it
props, // put props in local state so that getDerivedStateFromProps can access it
};
this._initialize(props);

this.fixedDataTableApi = createApi();
this.previousApiValue = null;
Expand Down Expand Up @@ -83,18 +72,57 @@ class FixedDataTableContainer extends React.Component {
};
}

componentDidMount() {
this._initialize(this.props);
}

componentWillUnmount() {
if (this.unsubscribe) {
this.unsubscribe();
this.unsubscribe = null;
}
this.reduxStore = null;
this._cleanup();
}

componentDidUpdate() {
this.notifyApiValueChanges();
}

_initialize(props) {
if (this.reduxStore) {
// already initialized
return;
}

this.reduxStore = FixedDataTableStore.get();
this.reduxStore.dispatch(initialize(props));

this.scrollActions = getScrollActions(this.reduxStore, () => this.props);

this.unsubscribe = this.reduxStore.subscribe(this.onStoreUpdate.bind(this));

const state = {
boundState: FixedDataTableContainer.getBoundState(this.reduxStore), // the state from the redux store
reduxStore: this.reduxStore, // put store instance in local state so that getDerivedStateFromProps can access it
props, // put props in local state so that getDerivedStateFromProps can access it
};

if (this.state) {
// component is already constructed, so just update state
this.setState(state);
} else {
// component isn't constructed yet, so initialize state
this.state = state;
}
}

_cleanup() {
if (!this.reduxStore) {
// already cleaned up
return;
}

this.unsubscribe();
this.unsubscribe = null;
this.reduxStore = null;
}

/**
* Returns FDT's public API.
*
Expand Down

0 comments on commit 3924dff

Please sign in to comment.