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

Instance.On listener is inconsistent with Lifecycle Events #357

Open
3 tasks
shafferchance opened this issue Feb 2, 2023 · 8 comments
Open
3 tasks

Instance.On listener is inconsistent with Lifecycle Events #357

shafferchance opened this issue Feb 2, 2023 · 8 comments

Comments

@shafferchance
Copy link

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 as instance.asyncOn.

Here is a link to a code sandbox demonstrating the inconsistent behaviour.

Acceptance Criteria

  • Async behaviour is consistent between on and plugin lifecycle functions
  • Typed appropriately
  • Tests demonstrating this behaviour is working as expected

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.

@DavidWells
Copy link
Owner

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

@coreybruyere
Copy link

@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.

@shafferchance
Copy link
Author

shafferchance commented Apr 5, 2023

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.
       }
   });
}

@coreybruyere
Copy link

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!

@shafferchance
Copy link
Author

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:

  1. All of these events are asynchronous, meaning if you want a dedicated order you should order them as such. Whenever you call track or page a promise is returned either await them in an async function or chain them.
  2. The order between your page-view event and track events shouldn't matter on the page.
    - Now in between your form it would and in that case I would use a queue-like system and essentially store either calls there so that you don't even initiate the second call till the first is resolved or chain their result so you handle them in order (this is only relevant if you care about the data post track/pageEnd)
  3. Perhaps you should add timestamps and rearrange wherever your data is being ingested at

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

@coreybruyere
Copy link

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:

  1. Fire step-1 page event on load
  2. Fire step-1 various tracking events for different fields as they're interacted with
  3. Fire step-1 SUBMIT tracking event form_submitted
  4. Once step-1 SUBMIT tracking event is resolved... Fire step-2 page event on load
  5. Fire step-2 various tracking events for different fields as they're interacted with
  6. Fire step-2SUBMIT tracking event form_submitted
  7. Once step-2 SUBMIT tracking event is resolved... Fire step-3 page event on load
  8. And so on...

Any help with this would be greatly appreciated. Thanks for the insight so far.

@shafferchance
Copy link
Author

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:

  1. All of these events are asynchronous, meaning if you want a dedicated order you should order them as such. Whenever you call track or page a promise is returned either await them in an async function or chain them.
  2. The order between your page-view event and track events shouldn't matter on the page.
    - Now in between your form it would and in that case I would use a queue-like system and essentially store either calls there so that you don't even initiate the second call till the first is resolved or chain their result so you handle them in order (this is only relevant if you care about the data post track/pageEnd)
  3. Perhaps you should add timestamps and rearrange wherever your data is being ingested at

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

@coreybruyere
Copy link

@shafferchance Can you provide an example for a queue-like system?

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

No branches or pull requests

3 participants