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

Client analytics refactorings, dead code removal #98

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

cxcorp
Copy link
Collaborator

@cxcorp cxcorp commented Jun 24, 2019

Dead code removal:

  • Removes old sendActions method and socket event handler which was used to delegate client commands to the display in the past. Nowadays the commands are not handled in the display, but on the server.

Client analytics refactorings (closes #99):

  • Refactors GoogleAnalytics.ts to either use the actual GA class or a dummy class depending on whether we're in a browser or on the server, instead of making the isBrowser call every time analytics is invoked.
  • Instead of preserving state in the display and client component about previous active views for analytics purposes, these now use Observables which make them easier to reason about.

Originally, the commands from the controller would be delegated to the
display which computed the game logic. Nowadays the logic is handled on
the server, meaning that the clients do not need to listen to the
command events at all - they only listen for "state" events.

Removed unnecessary socket event listener from withSocket, stuff
related to the old architecture from storybook, and the
SocketIODisplays::sendActions method from the server.
Previously the Display component's socket event type was filtered in
onSocket. Now that the only inbound message is "state", it can be
filtered in componentDidMount.
The previous lazy loading function would cause the first analytics event
to be lost because fn() was never called after ReactGA.intialize.

Instead of directly fixing that, I chose to initialize ReactGA in the
constructor instead.
Instead of checking isBrowser() inside the GoogleAnalytics class, the
_app component now just uses a factory method to create the correct GA
implementation. When in SSR (not in browser), a dummy analytics impl is
used.
Instead of hooking up a sendAnalytics call in render(), the "active view
changed" analytics event is now sent via an observable subscription.
@cxcorp cxcorp force-pushed the refactor-analytics-and-client branch from 5adef14 to d233286 Compare June 24, 2019 09:04
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.

First Google Analytics event is missed
1 participant