Open shafferchance opened 1 year ago
Thanks for the issue and repro! Very detailed!
Which part of the repro is inconsistent? 😅 Having trouble parsing that out of the console.logs of example
@shafferchance you mentioned workarounds. What workarounds have worked for you? I’m having issues with lifecycle events as well. Specifically, multi-step forms where their onSubmit tracking events are firing after page events. @DavidWells any insight on why that might be happening would be super helpful. I can provide some code samples and a better explanation of the issue a bit later.
Oh he, I totally lost this in the weeds.
@DavidWells I will make a demo with a more clear demonstration later on.
@coreybruyere How are you all processing the events, each "event" (track, identity, page, etc) all return a promise so if you want to explicitly order events you will need one to await the other or chain them.
As far as a workaround to instance.on
not being async I solved this by registering functions in my own table and using that within the plugin, registering and unregistering via event-name then id. With each call, I then invoke each function and await them all via Promise.all.
function plugin() {
const fnTable = new Map(); // Can be a plain {} too
return ({
name: "some-plugin",
methods: {
addFn: (event, fn) => {
if (fnTable.has(event)) {
fnTable.set(event, new Map());
}
const eventTable = fnTable.get(event);
const fnUUID = crypto.randomUUID(); // Only on new enough browsers
eventTable.set(fnUUID, fn); // I use Typescript to enforce it but each should be an async function or return a promise
},
rmFn: (event, uuid) => {
const eventTable = fnTable.get(event);
if (eventTable === undefined) {
return;
}
eventTable.delete(uuid);
}
},
// This same code can be used for any event
track: (params) => {
const { payload } = params;
const events = fnTable.get(payload.event); // Or somewhere else
if (events && events.size > 0) {
const jobs = [];
for (const fn of events.values()) {
jobs.push(fn(params));
}
return Promise.all(events); // If I remember correctly returning a promise is properly processed.
}
});
}
Thanks for the response @shafferchance. I'm having issues with events not firing in the correct order. Specifically, in a multi-step form.
In my case I'm tracking pages in the root of my app on location change:
// App.tsx
useEffect(() => {
// Track initial pageview and route change
analytics.page({
pathname: location.pathname,
});
}, [location]);
Then, in my multi step form I'm tracking each step:
// FormStep1.tsx
const formik = useFormik({
....
onSubmit: () => {
analytics.track("form_submitted", {
step: "Step 1",
});
...
},
});
<form onSubmit={formik.handleSubmit}>..</form>
I'm using Mixpanel for analytics and am having events come in a bit inconsistently.
Eg., The page event firing and tracking the page for FormStep2 followed by firing the event for the onSubmit for FormStep1, when the onSubmit should be first and the page event second. I think I need to spend some more time diving into lifecycle events in React but any insight is appreciated. Thanks!
I can't see all your code but I'm assuming the Multi-Step form is being brought into the App.tsx
.
There are a couple of things I want to note here:
track
or page
a promise is returned either await
them in an async function or chain them.These are around ensuring that the jobs are called in the order you expect, one to be called after the other, you will likely need to queue as aforementioned.
As for your react lifecycle comment, onSubmit will be fired when click the submit type input within your form your useEffect
should fire first but that depends on how location is location is passed in, I see you're using window.location
inside but it's not clear if that the dependency or not.
Let me know if this helps or if you need an example
Thanks @shafferchance
Yea, multi step form is coming from root App.tsx and location
from react-router-dom is being used as a dependency to the page event in the App.tsx useEffect.
const location = useLocation();
useEffect(() => {
// Track initial pageview and route change
analytics.page({
pathname: location.pathname,
});
}, [location]);
return (
<AnalyticsProvider instance={analytics}>
<Routes>
<Route path="form" element={<Form />}>
<Route path="step-1" element={<Step1 />} />
<Route path="step-2" element={<Step2 />} />
<Route path="step-3" element={<Step3 />} />
</Route>
</Routes>
</AnalyticsProvider>
I'm curious to see an example for the second item you listed if you have time. I wasn't aware of the different page events from the library and am wondering if something like that might resolve the discrepancy between pages and form submission events.
A queue-like system would make the most sense in this case, where after the final tracking event (a form step submitted) is resolved the next (page) event is fired. In this case, it would look something like:
form_submitted
form_submitted
Any help with this would be greatly appreciated. Thanks for the insight so far.
I can't see all your code but I'm assuming the Multi-Step form is being brought into the App.tsx
.
There are a couple of things I want to note here:
track
or page
a promise is returned either await
them in an async function or chain them.These are around ensuring that the jobs are called in the order you expect, one to be called after the other, you will likely need to queue as aforementioned.
As for your react lifecycle comment, onSubmit will be fired when click the submit type input within your form your useEffect
should fire first but that depends on how location is location is passed in, I see you're using window.location
inside but it's not clear if that the dependency or not.
Let me know if this helps or if you need an example
@shafferchance Can you provide an example for a queue-like system?
Description
Based on that the events: track, page, etc. work with async functions and wait as expected this behaviour should be consistent across the functions. Looking at the implementation for on it makes sense why the lack of support is present, but how could we world to establish either an
instance.on
that supports async or a new method such asinstance.asyncOn
.Here is a link to a code sandbox demonstrating the inconsistent behaviour.
Acceptance Criteria
If this is meant to be this way so be it, there are some workarounds, but curious how others felt about the inconsistency. Thank you for your time and if @DavidWells would like a hand making this change I would be open to making a PR and getting your support along the way.