Skip to content
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

Add logic to turn off network requests when Scope dismounts #2290

Merged
merged 2 commits into from
Mar 8, 2017

Conversation

jpellizzari
Copy link
Contributor

@jpellizzari jpellizzari commented Mar 1, 2017

Needed for https://github.com/weaveworks/service-ui/pull/184.

When navigating to another page in Weave Cloud, Scope will be unmounted by react-router. This change will turn off topology polling and websocket connections when Scope dismounts.

@jpellizzari
Copy link
Contributor Author

@davkal If you could review when you get a chance. I pushed up a commit on service-ui that does a LocalModuleProxy to a local Scope repo. You can test this change along with the scope as a component PR by doing:

SCOPE_PATH=/path/to/scope PROXY_SCOPE_COMPONENT=true npm start

in the service-ui repo.

@jpellizzari
Copy link
Contributor Author

jpellizzari commented Mar 3, 2017

Added some extra files to support doing partial imports, ie:
import reducer from 'weave-scope/reducer'

I originally attempted to avoid polluting the app/scripts directory with more files using a bash script when building the npm package...

#! /bin/bash
split -l 1 app/scripts/index.js ../tmp/module__
for file in `ls ../tmp/module__*`; do
  contents=$(cat $file)
  module_name=$(echo $contents | grep -oE 'exports\.(.*?\s)' | sed 's/exports\.//')
  filename="build-pkg/$(echo "$module_name.js" | sed 's/ //')"
  rm $file
  touch "$filename"
  echo $contents > "$filename"
done

...but that will not work with LocalModuleProxy.

Copy link
Contributor

@davkal davkal left a comment

Choose a reason for hiding this comment

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

Small code nits. Otherwise LGTM

clearTimeout(reconnectTimer);
if (socket) {
socket.onerror = null;
socket.onclose = null;

This comment was marked as abuse.

if (storageState) {
window.location.hash = `!/state/${storageState}`;
const parsedState = JSON.parse(decodeURL(storageState));
mergedState = Object.assign(initialState, parsedState);

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

@jpellizzari jpellizzari merged commit 743ead7 into master Mar 8, 2017
@jpellizzari jpellizzari deleted the cancel-polling branch March 21, 2017 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants