lahsivjar / react-stomp

React websocket component for STOMP protocol over SockJs
https://www.npmjs.com/package/react-stomp
MIT License
134 stars 41 forks source link

Should we move away from UNSAFE_componentWillReceiveProps #173

Open IMM9O opened 3 years ago

IMM9O commented 3 years ago

I want to address some adjustment improvement to remove the need for use UNSAFE_componentWillReceiveProps as it throws an error with react 17 and use shouldComponentUpdate method instance

and here my suggestion, it ok?

  shouldComponentUpdate (nextProps, nextState) {
    if (this.state.connected) {
      // Subscribe to new topics
      difference(nextProps.topics, this.props.topics).forEach((newTopic) => {
        this._log('Subscribing to topic: ' + newTopic)
        this._subscribe(newTopic)
      })

      // Unsubscribe from old topics
      difference(this.props.topics, nextProps.topics).forEach((oldTopic) => {
        this._log('Unsubscribing from topic: ' + oldTopic)
        this._unsubscribe(oldTopic)
      })
    }
    return false
  }
AnthonyNGarcia commented 3 years ago

Also seeing the same warning in browser console, would really like to see an update like this pulled in. An otherwise awesome tool here.

lahsivjar commented 3 years ago

Have been away from this project for a while, will take a look into this sometime next week.

IncPlusPlus commented 2 years ago

@lahsivjar Do you think this could be resolved if a pull request was made with the proposed changes? This could potentially close #133 and #195