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

Delay initialization (lazy loading) #637

Merged
merged 16 commits into from
Nov 1, 2022
Merged

Conversation

silesky
Copy link
Contributor

@silesky silesky commented Oct 24, 2022

import { AnalyticsBrowser } from '@segment/analytics-next'

const analytics = new AnalyticsBrowser() // nothing loaded yet

// queue events
analytics.track('foo')
analytics.identify('bar')

if (userConsentsToTracking) {
  analytics.load({ writeKey: 'foo' }) // loaded. queued events should now be sent.
}

@changeset-bot
Copy link

changeset-bot bot commented Oct 24, 2022

🦋 Changeset detected

Latest commit: b66c10f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@segment/analytics-next Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@silesky silesky changed the title WIP: Delay initialization WIP: Delay initialization (lazy load analytics) Oct 24, 2022
@silesky silesky changed the title WIP: Delay initialization (lazy load analytics) WIP/PoC: Delay initialization (lazy load analytics) Oct 24, 2022
@silesky silesky changed the title WIP/PoC: Delay initialization (lazy load analytics) WIP/PoC: Delay initialization (lazy loading) Oct 24, 2022
@silesky silesky force-pushed the feature/add-delayed-init-ability branch from bb8b8ab to 6d6dd29 Compare October 24, 2022 03:16
@silesky silesky force-pushed the feature/add-delayed-init-ability branch from 6d6dd29 to 22de419 Compare October 24, 2022 03:17
@silesky silesky force-pushed the feature/add-delayed-init-ability branch from 751759a to a67cfc2 Compare October 26, 2022 16:54
@zikaari
Copy link
Contributor

zikaari commented Oct 26, 2022

I'm curious wouldn't this have worked anyway? We've always buffered up events until load, and flushed them when loaded. Can you explain behavioral differences between the existing implementation and this one?

@silesky silesky force-pushed the feature/add-delayed-init-ability branch 2 times, most recently from 634ad90 to c1dbc13 Compare November 1, 2022 17:45
@silesky silesky force-pushed the feature/add-delayed-init-ability branch from c1dbc13 to 0894ef2 Compare November 1, 2022 17:46
@silesky silesky changed the title WIP/PoC: Delay initialization (lazy loading) Delay initialization (lazy loading) Nov 1, 2022
}

/**
* Starts loading an analytics instance including:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a note that this should only be called once? Want to make it clear that calling this with different settings won't essentially 'reload' analytics.

/**
* Return a promise that can be externally resolved
*/
export const createDeferred = <T>() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a better name than extractPromiseParts 😁

@@ -53,6 +53,29 @@ document.body?.addEventListener('click', () => {
})
```

## Lazy / Delayed Loading
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding this section!

@silesky silesky force-pushed the feature/add-delayed-init-ability branch from 8cce6d2 to 754e74c Compare November 1, 2022 20:27
@silesky silesky merged commit b335096 into master Nov 1, 2022
@silesky silesky deleted the feature/add-delayed-init-ability branch November 1, 2022 20:36
@github-actions github-actions bot mentioned this pull request Nov 1, 2022
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.

3 participants