feat: add registerPlugin so a plugin can be added after the client starts - #522
Open
abelonogov-ld wants to merge 1 commit into
Open
feat: add registerPlugin so a plugin can be added after the client starts#522abelonogov-ld wants to merge 1 commit into
abelonogov-ld wants to merge 1 commit into
Conversation
…arts Plugins could only be supplied through LDConfig, so an integration that learns about a plugin later — or that wants to instrument a client it did not configure — had no way in. Hooks were held in a constant array, which registration after start cannot extend, so they now live behind a lock and are replaced rather than mutated in place. Each series reads a snapshot once, so the hooks a series ends with are the hooks it began with: read again mid-series, a hook registered in between would be handed an "after" stage for a series whose "before" stage it was never in. Hooks go live only once register returns, matching the Android and .NET ordering, so a plugin's own hooks do not observe its register call. Retaining EnvironmentMetadata on the instance lets a plugin registered later be handed the same environment description as one configured up front, and removes the duplicate construction in start and collectHooks. Co-authored-by: Cursor <cursoragent@cursor.com>
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
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.
Summary
Plugins could only be supplied through
LDConfig, so an integration that learns about a plugin later — or that wants to instrument a client it did not configure — had no way in. This addsLDClient.registerPlugin(_:), matching the method the .NET and Flutter SDKs already expose and the one being added to the Android SDK in launchdarkly/android-client-sdk#393.private varbehind anNSLockand are replaced rather than mutated in place. Each series reads a snapshot once into a local, so the hooks a series ends with are the hooks it began with — were the array read again mid-series, a hook registered in between would be handed anafterstage for a series whosebeforestage it was never in. The three read sites (evaluateWithHooks,executeBeforeIdentifyHooks,executeAfterTrackHooks) all take that snapshot up front.registerreturns. This matches the Android and .NET ordering, and differs from configuration-time registration where a plugin's hooks are active beforeregisteris called. So a plugin's own hooks will not observe evaluations or identify calls itsregistermakes; everything afterwards does run them.EnvironmentMetadatais retained on the instance, so a plugin registered later is handed the same environment description as one configured up front. This also removes the duplicate construction that previously existed in bothstartandcollectHooks.Registration applies to the one client it is called on, so a multi-environment setup means calling it per environment.
Note that the iOS
Pluginprotocol has noonPluginsReady, so unlike Android this path is justgetHooksthenregisterthen activate.Test plan
Five cases added to
LDClientPluginsSpec, all passing on an iPhone 16e simulator alongside the two pre-existing plugin tests (7 total):testRegisterPluginPassesClientAndEnvironmentMetadatatestRegisterPluginActivatesBundledHookstestRegisterPluginDoesNotRunTheRegisteringPluginsOwnHookstestRegisterPluginHooksRunAfterConfiguredHookstestRegisterPluginAppliesOnlyToTheClientItIsCalledOnNote
Overview
Adds
LDClient.registerPlugin(_:)so a plugin can be attached after start, not only viaLDConfig.plugins. Registration is per client (call it on each environment in a multi-env setup).Hooks are no longer a constant array: they sit behind an
NSLock, and each evaluation/identify/track series snapshots them once so a hook registered mid-series never gets anafterwithout a matchingbefore.registerruns before those hooks go live, so a plugin’s own hooks do not observe work done insideregister.EnvironmentMetadatais stored on the client so late plugins get the same description as config-time ones.Reviewed by Cursor Bugbot for commit 5798e94. Bugbot is set up for automated code reviews on this repo. Configure here.