Merge pull request #1802 from bugsnag/PLAT-8735-preserve-feature-flag…

…-order

[PLAT-8735] Preserve feature flag order

CHANGELOG.md

@@ -6,6 +6,7 @@
 
 - (react-native) Update bugsnag-cocoa from v6.20.0 to [v6.22.2](https://github.com/bugsnag/bugsnag-cocoa/blob/master/CHANGELOG.md#6222-2022-08-17)
 - (react-native) Update bugsnag-android from v5.24.0 to [v5.26.0](https://github.com/bugsnag/bugsnag-android/blob/master/CHANGELOG.md#5260-2022-08-18)
+- Refactor feature flags to maintain insertion order [#1802](https://github.com/bugsnag/bugsnag-js/pull/1802)
 
 ## v7.17.3 (2022-07-18)
 

packages/core/client.d.ts

@@ -1,4 +1,4 @@
-import { Client, OnErrorCallback, Config, Breadcrumb, Session, OnSessionCallback, OnBreadcrumbCallback, Plugin, Device, App, User } from './types'
+import { Client, OnErrorCallback, Config, Breadcrumb, Session, OnSessionCallback, OnBreadcrumbCallback, Plugin, Device, App, User, FeatureFlag } from './types'
 import EventWithInternals from './event'
 
 interface LoggerConfig {
@@ -43,14 +43,14 @@ export default class ClientWithInternals<T extends Config = Config> extends Clie
   _config: T
   _depth: number
   _logger: LoggerConfig
-  _breadcrumbs: Breadcrumb[];
+  _breadcrumbs: Breadcrumb[]
   _delivery: Delivery
   _setDelivery: (handler: (client: Client) => Delivery) => void
 
   _user: User
 
   _metadata: { [key: string]: any }
-  _features: { [key: string]: string | null }
+  _features: FeatureFlag | null[]
 
   startSession(): ClientWithInternals
   resumeSession(): ClientWithInternals

packages/core/client.js

@@ -9,10 +9,10 @@ const reduce = require('./lib/es-utils/reduce')
 const keys = require('./lib/es-utils/keys')
 const assign = require('./lib/es-utils/assign')
 const runCallbacks = require('./lib/callback-runner')
-const featureFlagDelegate = require('./lib/feature-flag-delegate')
 const metadataDelegate = require('./lib/metadata-delegate')
 const runSyncCallbacks = require('./lib/sync-callback-runner')
 const BREADCRUMB_TYPES = require('./lib/breadcrumb-types')
+const { add, clear, merge } = require('./lib/feature-flag-delegate')
 
 const noop = () => {}
 
@@ -36,7 +36,8 @@ class Client {
     this._breadcrumbs = []
     this._session = null
     this._metadata = {}
-    this._features = {}
+    this._featuresIndex = {}
+    this._features = []
     this._context = undefined
     this._user = {}
 
@@ -90,19 +91,20 @@ class Client {
   }
 
   addFeatureFlag (name, variant = null) {
-    featureFlagDelegate.add(this._features, name, variant)
+    add(this._features, this._featuresIndex, name, variant)
   }
 
   addFeatureFlags (featureFlags) {
-    featureFlagDelegate.merge(this._features, featureFlags)
+    merge(this._features, featureFlags, this._featuresIndex)
   }
 
   clearFeatureFlag (name) {
-    delete this._features[name]
+    clear(this._features, this._featuresIndex, name)
   }
 
   clearFeatureFlags () {
-    this._features = {}
+    this._features = []
+    this._featuresIndex = {}
   }
 
   getContext () {
@@ -151,7 +153,7 @@ class Client {
 
     // update and elevate some options
     this._metadata = assign({}, config.metadata)
-    featureFlagDelegate.merge(this._features, config.featureFlags)
+    merge(this._features, config.featureFlags, this._featuresIndex)
     this._user = assign({}, config.user)
     this._context = config.context
     if (config.logger) this._logger = config.logger
@@ -295,9 +297,9 @@ class Client {
     })
     event.context = event.context || this._context
     event._metadata = assign({}, event._metadata, this._metadata)
-    event._features = assign({}, event._features, this._features)
     event._user = assign({}, event._user, this._user)
     event.breadcrumbs = this._breadcrumbs.slice()
+    merge(event._features, this._features, event._featuresIndex)
 
     // exit early if events should not be sent on the current releaseStage
     if (this._config.enabledReleaseStages !== null && !includes(this._config.enabledReleaseStages, this._config.releaseStage)) {

packages/core/event.d.ts

@@ -1,4 +1,4 @@
-import { App, Device, Event, Request, Breadcrumb, User, Session } from './types'
+import { App, Device, Event, Request, Breadcrumb, User, Session, FeatureFlag } from './types'
 import { Error } from './types/event'
 
 interface HandledState {
@@ -21,7 +21,8 @@ interface FeatureFlagPayload {
 export default class EventWithInternals extends Event {
   constructor (errorClass: string, errorMessage: string, stacktrace: any[], handledState?: HandledState, originalError?: Error)
   _metadata: { [key: string]: any }
-  _features: { [key: string]: string | null }
+  _features: FeatureFlag | null[]
+  _featuresIndex: { [key: string]: number }
   _user: User
   _handledState: HandledState
   _session?: Session

packages/core/event.js

@@ -28,7 +28,8 @@ class Event {
     this.threads = []
 
     this._metadata = {}
-    this._features = {}
+    this._features = []
+    this._featuresIndex = {}
     this._user = {}
     this._session = undefined
 
@@ -56,19 +57,20 @@ class Event {
   }
 
   addFeatureFlag (name, variant = null) {
-    featureFlagDelegate.add(this._features, name, variant)
+    featureFlagDelegate.add(this._features, this._featuresIndex, name, variant)
   }
 
   addFeatureFlags (featureFlags) {
-    featureFlagDelegate.merge(this._features, featureFlags)
+    featureFlagDelegate.merge(this._features, featureFlags, this._featuresIndex)
   }
 
   clearFeatureFlag (name) {
-    delete this._features[name]
+    featureFlagDelegate.clear(this._features, this._featuresIndex, name)
   }
 
   clearFeatureFlags () {
-    this._features = {}
+    this._features = []
+    this._featuresIndex = {}
   }
 
   getUser () {

packages/core/lib/clone-client.js

@@ -9,7 +9,8 @@ module.exports = (client) => {
   // so ensure they are are (shallow) cloned
   clone._breadcrumbs = client._breadcrumbs.slice()
   clone._metadata = assign({}, client._metadata)
-  clone._features = assign({}, client._features)
+  clone._features = [...client._features]
+  clone._featuresIndex = assign({}, client._featuresIndex)
   clone._user = assign({}, client._user)
   clone._context = client._context
 

packages/core/lib/feature-flag-delegate.js

@@ -1,9 +1,9 @@
 const map = require('./es-utils/map')
-const keys = require('./es-utils/keys')
+const filter = require('./es-utils/filter')
 const isArray = require('./es-utils/is-array')
 const jsonStringify = require('@bugsnag/safe-json-stringify')
 
-function add (existingFeatures, name, variant) {
+function add (existingFeatures, existingFeatureKeys, name, variant) {
   if (typeof name !== 'string') {
     return
   }
@@ -14,10 +14,17 @@ function add (existingFeatures, name, variant) {
     variant = jsonStringify(variant)
   }
 
-  existingFeatures[name] = variant
+  const existingIndex = existingFeatureKeys[name]
+  if (typeof existingIndex === 'number') {
+    existingFeatures[existingIndex] = { name, variant }
+    return
+  }
+
+  existingFeatures.push({ name, variant })
+  existingFeatureKeys[name] = existingFeatures.length - 1
 }
 
-function merge (existingFeatures, newFeatures) {
+function merge (existingFeatures, newFeatures, existingFeatureKeys) {
   if (!isArray(newFeatures)) {
     return
   }
@@ -30,27 +37,37 @@ function merge (existingFeatures, newFeatures) {
     }
 
     // 'add' will handle if 'name' doesn't exist & 'variant' is optional
-    add(existingFeatures, feature.name, feature.variant)
+    add(existingFeatures, existingFeatureKeys, feature.name, feature.variant)
   }
+
+  return existingFeatures
 }
 
 // convert feature flags from a map of 'name -> variant' into the format required
 // by the Bugsnag Event API:
 //   [{ featureFlag: 'name', variant: 'variant' }, { featureFlag: 'name 2' }]
 function toEventApi (featureFlags) {
   return map(
-    keys(featureFlags),
-    name => {
+    filter(featureFlags, Boolean),
+    ({ name, variant }) => {
       const flag = { featureFlag: name }
 
       // don't add a 'variant' property unless there's actually a value
-      if (typeof featureFlags[name] === 'string') {
-        flag.variant = featureFlags[name]
+      if (typeof variant === 'string') {
+        flag.variant = variant
       }
 
       return flag
     }
   )
 }
 
-module.exports = { add, merge, toEventApi }
+function clear (features, featuresIndex, name) {
+  const existingIndex = featuresIndex[name]
+  if (typeof existingIndex === 'number') {
+    features[existingIndex] = null
+    delete featuresIndex[name]
+  }
+}
+
+module.exports = { add, clear, merge, toEventApi }

packages/core/lib/test/clone-client.test.ts

@@ -116,14 +116,14 @@ describe('clone-client', () => {
 
     const clone = cloneClient(original)
 
-    expect(clone._features).toStrictEqual({ abc: '123' })
+    expect(clone._features).toStrictEqual([{ name: 'abc', variant: '123' }])
     expect(clone._features).not.toBe(original._features)
 
     // changing the clone's feature flags shouldn't affect the original
     clone.addFeatureFlag('xyz', '999')
 
-    expect(clone._features).toStrictEqual({ abc: '123', xyz: '999' })
-    expect(original._features).toStrictEqual({ abc: '123' })
+    expect(clone._features).toStrictEqual([{ name: 'abc', variant: '123' }, { name: 'xyz', variant: '999' }])
+    expect(original._features).toStrictEqual([{ name: 'abc', variant: '123' }])
   })
 
   it('should clone user information', () => {