feat: Add support for multiple context event summaries. #821
Conversation
| @@ -122,7 +122,7 @@ export class BrowserClient extends LDClientImpl implements LDClient { | |||
| clientSideId, | |||
| autoEnvAttributes, | |||
| platform, | |||
| filterToBaseOptions({ ...options, logger }), | |||
There was a problem hiding this comment.
Unrelated bug, will be fixed before this goes forward.
|
@launchdarkly/js-client-sdk-common size report |
| @@ -12,16 +12,16 @@ describe('given an event summarizer', () => { | |||
| }); | |||
|
|
|||
| it('does nothing for an identify event.', () => { | |||
| const beforeSummary = summarizer.getSummary(); | |||
| const beforeSummary = summarizer.getSummaries()[0]; | |||
There was a problem hiding this comment.
These tests are just updated to handle the API difference added by supporting multiple summaries.
| @@ -1,14 +1,25 @@ | |||
| import { Hasher } from '../src/api'; | |||
|
|
|||
| class MockHasher implements Hasher { | |||
There was a problem hiding this comment.
The mock hasher implementation that was being used for client-side was not sufficiently representative of a real hash.
| @@ -144,4 +144,8 @@ export default class AttributeReference { | |||
| this._components.every((value, index) => value === other.getComponent(index)) | |||
| ); | |||
| } | |||
|
|
|||
| public get components() { | |||
There was a problem hiding this comment.
This is only public to within the SDK.
There was a problem hiding this comment.
The event summarizer needed an interface to make it swappable for client/server.
| @@ -143,6 +146,15 @@ export default class EventProcessor implements LDEventProcessor { | |||
| _config.privateAttributes.map((ref) => new AttributeReference(ref)), | |||
| ); | |||
|
|
|||
| if (summariesPerContext) { | |||
There was a problem hiding this comment.
We theoretically may want to move this up the stack and use dependency injection instead. Though it will require some shuffling of things around.
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
| })(); | ||
| } | ||
|
|
||
| getSummaries(): SummarizedFlagsEvent[] { |
There was a problem hiding this comment.
Moving to a model where getting the summaries automatically clears. It makes it easier to validate there are no race conditions from async interactions.
| @@ -70,6 +70,7 @@ | |||
| "packageManager": "yarn@3.4.1", | |||
| "//": "Pin jsonc-parser because v3.3.0 breaks rollup-plugin-esbuild", | |||
| "resolutions": { | |||
| "jsonc-parser": "3.2.0" | |||
| "jsonc-parser": "3.2.0", | |||
| "parse5": "7.2.1" | |||
There was a problem hiding this comment.
Pin parse5 because it currently has a typescript problem.
|
Closed to improve hash. |
No description provided.