fix(browser): Improve extension lifecycle for tree shaking#3164
Merged
dustinbyrne merged 5 commits intomainfrom Mar 3, 2026
Merged
fix(browser): Improve extension lifecycle for tree shaking#3164dustinbyrne merged 5 commits intomainfrom
dustinbyrne merged 5 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
749
to
+759
| if (ext.webVitalsAutocapture) { | ||
| initTasks.push(() => { | ||
| this.webVitalsAutocapture = new ext.webVitalsAutocapture!(this) as WebVitalsAutocapture | ||
| }) | ||
| this.extensions.push((this.webVitalsAutocapture = new ext.webVitalsAutocapture(this))) | ||
| } | ||
|
|
||
| if (ext.exceptionObserver) { | ||
| initTasks.push(() => { | ||
| this.exceptionObserver = new ext.exceptionObserver!(this) as ExceptionObserver | ||
| this.exceptionObserver.startIfEnabledOrStop() | ||
| }) | ||
| this.extensions.push((this.exceptionObserver = new ext.exceptionObserver(this))) | ||
| } | ||
|
|
||
| if (ext.deadClicksAutocapture) { | ||
| this.extensions.push( | ||
| (this.deadClicksAutocapture = new ext.deadClicksAutocapture(this, isDeadClicksEnabledForAutocapture)) | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
WebVitalsAutocapture, ExceptionObserver, and DeadClicksAutocapture don't have initialize() methods and call their startup logic in constructors. This means they initialize immediately (synchronously) when constructed here, unlike other extensions which have deferred initialization via initialize() being called in initTasks (lines 761-766).
This creates inconsistent initialization timing across extensions and changes behavior compared to the old code where their construction was deferred via initTasks. Consider adding initialize() methods to these three classes (moving startup logic out of constructors) to ensure all extensions follow the unified lifecycle pattern.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Contributor
Author
There was a problem hiding this comment.
This is fine; I don't see any reason to need to change this. Calls to initialize() are guarded anyway, it won't call it if it doesn't have an initialize function.
Contributor
Contributor
|
Size Change: -2.75 kB (-0.04%) Total Size: 6.71 MB
ℹ️ View Unchanged
|
dustinbyrne
changed the title
19 tasks
19 tasks
Member
|
pr has a conflict otherwise all good |
marandaneto
approved these changes
Mar 2, 2026
18 tasks
Introduce an Extension interface with optional initialize() and onRemoteConfig() lifecycle hooks. Existing conditionally-constructed extensions now implement this interface and are tracked in an extensions[] array, enabling unified lifecycle dispatch.
dustinbyrne
force-pushed
the
feat/extension-lifecycle
branch
from
918b72e to
570817b
Compare
dustinbyrne
had a problem deploying
to
NPM Release
GitHub Actions
Failure
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Extensions in posthog-js are tree-shakable via
__extensionClasses, but the lifecycle is ad-hoc. Each extension has its own startup pattern (startIfEnabled(),startIfEnabledOrStop(),init(),loadIfEnabled(), etc.), and_onRemoteConfigdispatches to each one with individual hardcoded calls. This means posthog-core has to know about every extension individually, and adding or removing one requires touching multiple places. A unified interface lets us treat all extensions equally and call them in a loop.This is PR 1/3
Changes
Introduces an
Extensioninterface with standardized lifecycle hooks (initialize()andonRemoteConfig()) and anextensions: Extension[]array on the PostHog class.Extensioninterface (extensions/types.ts): Definesinitialize?()andonRemoteConfig?(config)hooks, plus anExtensionConstructor<T>type replacing the oldExtension<T>alias in types.ts._initExtensionspushes constructed extensions to the array and callsinitialize()._onRemoteConfigiterates the array instead of calling each extension individually (surveys, logs, conversations, and exceptions still use hardcoded calls since they aren't tree-shakable yet)._replaceExtensionhelper: Used when session recording is recreated during cookieless opt-in — removes old instance from array and initializes the new one.implement Extensionwith aninitialize()method wrapping their existing startup logic.ENTRYenv var to filter entrypoints during build.No behavioral changes. This is a structural refactor of code that already worked via
__extensionClasses.Bundle size
Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file