Integrate Heap analytics into the website #866
Conversation
| this._heap = heap; | ||
| } | ||
| // HeapAnalytics Wrappers | ||
| public async indentifyAsync(id: string, idType: string): Promise<void> { |
There was a problem hiding this comment.
Similar to my comment in the sourcemaps PR, is there a clean way to isolate the no floating promises rule to this file and change the api to be non async? It will be less confusing that way, so it is explicit that the code does not need to be awaited
| @@ -313,14 +313,13 @@ export const utils = { | |||
| const baseUrl = `https://${window.location.hostname}${hasPort ? `:${port}` : ''}`; | |||
| return baseUrl; | |||
| }, | |||
| async onPageLoadAsync(): Promise<void> { | |||
| onPageLoadPromise: new Promise((resolve, _reject) => { | |||
There was a problem hiding this comment.
Hmm this is a bit inconsistent with how we normally handle promises, is this functionally different than creating a new promise each time? Was the main issue that resolve wasn't being called in the if statement?
There was a problem hiding this comment.
The issue is that you can't set window.onload = resolve multiple times (ie. for multiple promises, a new one created for each function call) because window.onload can only be one function and is only called once. So with the previous method here is what would happen:
window.onload = A
window.onload = B
window.onload = C
window.onload() // C is called
The Promises made for A and B are never resolves and await forever. Imho this is the best way to do it.
There was a problem hiding this comment.
I see, are we at risk of never resolving if we set window.onload too late?
There was a problem hiding this comment.
Previously, the only onPageLoadAsync() that worked was the last call, all previous Promises that were returned will never resolve because window.onload points to the latest resolve function. Calling onPageLoadAsync() after window.onload worked fine because we checked for that and returned.
| @@ -112,6 +113,9 @@ export class SubscribeForm extends React.Component<SubscribeFormProps, Subscribe | |||
| try { | |||
| const response = await backendClient.subscribeToNewsletterAsync(this.state.emailText); | |||
| const status = response.status === 200 ? SubscribeFormStatus.Success : SubscribeFormStatus.Error; | |||
| if (status === SubscribeFormStatus.Success) { | |||
| analytics.indentify(this.state.emailText, 'email'); | |||
Description
I basically removed react-ga, included Heap (which is not available as a UMD module) and replaced all the calls to GA with calls to Heap.
I increased the amount of data we are logging and improved the way we are logging the data since Heap provides a better / more expansive API.
https://docs.heapanalytics.com/reference#addeventproperties.
I also fixed
onPageLoadAsyncwhich wasn't actually working.Testing instructions
Play around with the website and go to our heap dashboard (I can send invite) and see how things are going! The main reason we want this is for the SQL feature though.
Types of changes
Checklist: