Re-wrote Component to be more modern#14
Re-wrote Component to be more modern#14jediofthecode wants to merge 11 commits intocpunion:masterfrom
Conversation
… hooks and better tooling"
|
This requires a bit more work, but I made the PR to propose the new structure. Otherwise, I have package this as |
src/ActionCableProvider.jsx
Outdated
| this.componentWillUnmount(); | ||
|
|
||
| // create or assign cable | ||
| this.componentWillMount(); |
There was a problem hiding this comment.
componentWillMount use this.props, but in this context, must re-subscribe with nextProps
package.json
Outdated
| { | ||
| "name": "react-actioncable-provider", | ||
| "version": "1.0.3", | ||
| "name": "react-actioncable-provider-next", |
There was a problem hiding this comment.
Don't update name, urls in PR, you can add your name as contributors.
There was a problem hiding this comment.
Apologies, this was only to publish the package with a new name, as I needed to include it in a project im working on. I will revert this and push on my branch.
src/ActionCableProvider.jsx
Outdated
| class ActionCableProvider extends Component { | ||
| constructor(props) { | ||
| super(props); | ||
| this.displayName = 'ActionCableProvider'; |
There was a problem hiding this comment.
I think the instance variable displayName is unused.
- Added support for updating channel name and room on
component(Component will now automatically re-initialize cable
connection)
- Improved documentation surrounding new format
- Reverted package.json to older values
- Update ActionCableProvider component with suggested fixes
|
All the requested changes have been made, and some additional improvements that were necesary. |
|
Thanks! I will review and test later. |
| } | ||
| Cable.propTypes = { | ||
| channel: PropTypes.string.isRequired, | ||
| room: PropTypes.string |
There was a problem hiding this comment.
room is the room parameter that can be passed into the channel 'controller', it is from the ActionCable guide.
| import ActionCableProvider, { cable } from '../index'; | ||
|
|
||
| test('Render wrapped component without children', () => { | ||
| const Wrapped = cable(<div />); |
There was a problem hiding this comment.
I think the first parameter of cable should be a Component, not an instance.
| } | ||
|
|
||
| render () { | ||
| render() { |
There was a problem hiding this comment.
Can't use ActionCable component in the following anymore.
| config.room = room; | ||
| } | ||
|
|
||
| this.cable = this.context.cable.subscriptions.create(config, eventHandlers); |
There was a problem hiding this comment.
Rename this.cable to this.subscriptions more accurate.
| } | ||
|
|
||
| // cable is created by self, disconnect it | ||
| this.componentWillUnmount(); |
|
Thanks for your great work. That's big changes, I need more time to test. |
|
Sorry for the delay, i have been quite busy. I will try and get these changes in this, and I made some other fixes in the mean time that were crucial. |
2 participants
I re-wrote the base Provider component to use the more modern syntax. I also re-write the component completely, to be a HOC instead. I also improved the build process of the plugin itself. The build process requires some review, I used the
installhook, but i believe it is better to use thepreparehook instead. This require update of the documentation as well, I can do this when i have a bit more free time.To Do: