Skip to content
This repository was archived by the owner on Aug 13, 2018. It is now read-only.

Toggle Protocols #58

Merged
merged 3 commits into from
May 30, 2016
Merged

Toggle Protocols #58

merged 3 commits into from
May 30, 2016

Conversation

eliihen
Copy link
Member

@eliihen eliihen commented May 21, 2016

What

This PR adds a new feature - the ability to selectively enable or disable support for websocket protocols. This is added to the extension preferences window using the simple-prefs API.

Why

There are a few downsides to having all protocols enabled at all times.

  1. Performance. Having to try to parse every single frame into a plethora of protocols will kill perfomance in high-load situations. It is advantageous to be able to remove some or all protocols to allow for performant behaviour.
  2. Unused protocols. Most of the time, you are only interested in the protocol you and your team are actually using. In my case I am using a JSON based system, yet subscribe frames, which look like subscribe?..., are parsed by the parser as MQTT, which is not what I expect it to do

How

I added a new store and changed redux to be populated with the prefs at load. I also use the onPrefChanged hook to update the store on demand. Changing a setting is not retroactive, i.e. it will not change any already parsed frames, only future frames. I do not currently see this as an issue worth the complexity to fix.

screen shot 2016-05-21 at 23 44 24

@@ -82,7 +82,8 @@ var FrameList = React.createClass({
key: "frame-" + frame.id,
frame: frame,
selection: this.props.selection,
dispatch: this.props.dispatch
dispatch: this.props.dispatch,
config: this.props.dispatch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be: config: this.props.config

Honza

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right. Thanks

@janodvarko
Copy link
Member

Nice job here!

I made a few comments, please fix and I'll merge it into the master and release a new WSM version.

Honza

@eliihen
Copy link
Member Author

eliihen commented May 23, 2016

Thanks for the review. I'll fix your comments later today.

@eliihen
Copy link
Member Author

eliihen commented May 23, 2016

@janodvarko

All righty, I have now implemented the changes you requested

@janodvarko janodvarko merged commit 312f977 into firebug:master May 30, 2016
@janodvarko
Copy link
Member

Awesome, thanks!

Honza

@eliihen eliihen deleted the protocol-toggle branch June 6, 2016 19:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants