-
Notifications
You must be signed in to change notification settings - Fork 406
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
feat(websocket-plugin): add WebSocketDisconnected, WebsocketMessageError actions #825
Conversation
…ebsocketMessageError action not triggering
…ket disconnect fix to WebsocketMessageError action not triggering
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked and there is a lot of inconsistency all over, not related to your PR.
The main issue is that the plugin's module and options are written with lower s
. (ie NgxsWebsocketPluginModule)
If we want to deal with it, it should be fixed for the next major version.
@markwhitfeld @splincode @arturovt Any thoughts?
@eranshmil my opinion is we have to release the next version as soon as we can, because there a lot of features awaited by people. What do you think? |
We could merge this PR without the |
@eranshmil Yeah, probably, also could you please look with Mark at the edited markdown file, maybe there is some inconsistency in descriptions? |
I agree, this PR is good to go other than the rename. |
@markwhitfeld this is reverted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic! Thanks so much for this PR.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behaviour?
Does this PR introduce a breaking change?
Other information
#820