Refactored out unsafe_componentWillMount #1388
Conversation
|
|
||
| UNSAFE_componentWillMount() { | ||
| this.initializeTable(this.props); |
There was a problem hiding this comment.
initializeTable has changed the position of componentDidMount.
There was a problem hiding this comment.
Nope,
I left a trace while reading the code. Sorry. 🙏
| } | ||
|
|
||
| componentDidMount() { | ||
| this.setHeadResizeable(this.resizeHeadCellRefs, this.tableRef); | ||
|
|
||
| // When we have a search, we must reset page to view it unless on serverSide since paging is handled by the user. | ||
| if (this.props.options.searchText && !this.props.options.serverSide) this.setState({ page: 0 }); | ||
|
|
||
| this.setTableInit('tableInitialized'); |
There was a problem hiding this comment.
But shouldn't this be the first thing to do?
componentDidMount() {
this.setTableInit('tableInitialized');
...
}There was a problem hiding this comment.
To keep consistent, it probably should. I'll move it up.
| @@ -677,14 +670,15 @@ class MUIDataTable extends React.Component { | |||
| return transformedData; | |||
| }; | |||
|
|
|||
| setTableData(props, status, dataUpdated, callback = () => {}) { | |||
| setTableData(props, status, dataUpdated, callback = () => {}, fromConstructor = false) { | |||
There was a problem hiding this comment.
It doesn't seem necessary to add fromConstructor.
Because it can be judged by the existence of callback.
And I think it's better to separate the function.
I think it has too heavy functions.
There was a problem hiding this comment.
The constructor sets a default function for that callback. I also wanted to make it explicit at what was happening, since the constructor is the only place where the unique behavior happens.
There was a problem hiding this comment.
The constructor sets a default function for that callback. I also wanted to make it explicit at what was happening, since the constructor is the only place where the unique behavior happens.
Oh, I see. There is a default. hmmm...
|
It is good to remove unsafe_componentWillMount. it is such a great work! |
|
Now that I've been thinking about it, it's a refectoring, so you'd better focus logic. You can take the above reviews lightly. thanks! |
The table currently uses the method unsafe_componentWillMount in the main component. React is removing this lifecycle method in version 17 and it currently gives a warning message to developers using strict mode (#1261). The method is deemed unsafe because it can lead to memory leaks if not used properly.
React recommends using the constructor is initialize state or componentDidMount for initialization that requires DOM nodes. Only the former is relevant to mui-datatables, so the code has been refactored to use the constructor.