- Notifications
You must be signed in to change notification settings - Fork 201
feat(flags): add evaluation_environments to node and react-native SDKs #2417
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| --- | ||
| '@posthog/core': minor | ||
| --- | ||
| | ||
| feat: Add evaluation environments support for feature flags | ||
| | ||
| This PR adds base support for evaluation environments in the core library, allowing SDKs that extend the core to specify which environment tags their SDK instance should use when evaluating feature flags. | ||
| | ||
| The core library now handles sending the `evaluation_environments` parameter to the feature flags API when configured. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| --- | ||
| 'posthog-node': minor | ||
| --- | ||
| | ||
| feat: Add evaluation environments support for feature flags | ||
| | ||
| This PR implements support for evaluation environments in the posthog-node SDK, allowing users to specify which environment tags their SDK instance should use when evaluating feature flags. | ||
| | ||
| Users can now configure the SDK with an `evaluationEnvironments` option: | ||
| | ||
| ```typescript | ||
| const client = new PostHog('api-key', { | ||
| host: 'https://app.posthog.com', | ||
| evaluationEnvironments: ['production', 'backend', 'api'], | ||
| }) | ||
| ``` | ||
| | ||
| When set, only feature flags that have at least one matching evaluation tag will be evaluated for this SDK instance. Feature flags with no evaluation tags will always be evaluated. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| --- | ||
| 'posthog-react-native': minor | ||
| --- | ||
| | ||
| feat: Add evaluation environments support for feature flags | ||
| | ||
| This PR implements support for evaluation environments in the posthog-react-native SDK, allowing users to specify which environment tags their SDK instance should use when evaluating feature flags. | ||
| | ||
| Users can now configure the SDK with an `evaluationEnvironments` option: | ||
| | ||
| ```typescript | ||
| const posthog = new PostHog('api-key', { | ||
| host: 'https://app.posthog.com', | ||
| evaluationEnvironments: ['production', 'mobile', 'react-native'], | ||
| }) | ||
| ``` | ||
| | ||
| When set, only feature flags that have at least one matching evaluation tag will be evaluated for this SDK instance. Feature flags with no evaluation tags will always be evaluated. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,15 @@ | ||
| import { PostHog } from '@/entrypoints/index.node' | ||
| import { PostHog, PostHogOptions } from '@/entrypoints/index.node' | ||
| import { anyFlagsCall, anyLocalEvalCall, apiImplementation, isPending, wait, waitForPromises } from './utils' | ||
| import { randomUUID } from 'crypto' | ||
| | ||
| jest.mock('../version', () => ({ version: '1.2.3' })) | ||
| | ||
| const mockedFetch = jest.spyOn(globalThis, 'fetch').mockImplementation() | ||
| | ||
| const posthogImmediateResolveOptions: PostHogOptions = { | ||
| fetchRetryCount: 0, | ||
| } | ||
| | ||
| const waitForFlushTimer = async (): Promise<void> => { | ||
| await waitForPromises() | ||
| // To trigger the flush via the timer | ||
| | @@ -2467,6 +2471,92 @@ describe('PostHog Node.js', () => { | |
| }) | ||
| }) | ||
| | ||
| describe('evaluation environments', () => { | ||
| beforeEach(() => { | ||
| mockedFetch.mockClear() | ||
| }) | ||
| | ||
| it('should send evaluation environments when configured', async () => { | ||
| mockedFetch.mockImplementation( | ||
| apiImplementation({ | ||
| decideFlags: { 'test-flag': true }, | ||
| flagsPayloads: {}, | ||
| }) | ||
| ) | ||
| | ||
| const posthogWithEnvs = new PostHog('TEST_API_KEY', { | ||
| host: 'http://example.com', | ||
| evaluationEnvironments: ['production', 'backend'], | ||
| ...posthogImmediateResolveOptions, | ||
| Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider extracting Context Used: Context from Prompt To Fix With AIThis is a comment left during a code review. Path: packages/node/src/__tests__/posthog-node.spec.ts Line: 2483:2483 Comment: **style:** Consider extracting `posthogImmediateResolveOptions` to a shared constant at the top of the file for better maintainability **Context Used:** Context from `dashboard` - When defining constants, such as template IDs, use a constants file or define them at the top of the... ([source](https://app.greptile.com/review/custom-context?memory=046b860e-6d45-4e24-be8e-6e1727a1b550)) How can I resolve this? If you propose a fix, please make it concise. | ||
| }) | ||
| | ||
| await posthogWithEnvs.getAllFlags('some-distinct-id') | ||
| | ||
| expect(mockedFetch).toHaveBeenCalledWith( | ||
| 'http://example.com/flags/?v=2&config=true', | ||
| expect.objectContaining({ | ||
| method: 'POST', | ||
| body: expect.stringContaining('"evaluation_environments":["production","backend"]'), | ||
| Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: This string assertion is fragile. Consider using Prompt To Fix With AIThis is a comment left during a code review. Path: packages/node/src/__tests__/posthog-node.spec.ts Line: 2492:2492 Comment: **style:** This string assertion is fragile. Consider using `JSON.parse()` on the body and asserting against the parsed object structure instead How can I resolve this? If you propose a fix, please make it concise. | ||
| }) | ||
| ) | ||
| | ||
| await posthogWithEnvs.shutdown() | ||
| }) | ||
| | ||
| it('should not send evaluation environments when not configured', async () => { | ||
| mockedFetch.mockImplementation( | ||
| apiImplementation({ | ||
| decideFlags: { 'test-flag': true }, | ||
| flagsPayloads: {}, | ||
| }) | ||
| ) | ||
| | ||
| const posthogWithoutEnvs = new PostHog('TEST_API_KEY', { | ||
| host: 'http://example.com', | ||
| ...posthogImmediateResolveOptions, | ||
| }) | ||
| | ||
| await posthogWithoutEnvs.getAllFlags('some-distinct-id') | ||
| | ||
| expect(mockedFetch).toHaveBeenCalledWith( | ||
| 'http://example.com/flags/?v=2&config=true', | ||
| expect.objectContaining({ | ||
| method: 'POST', | ||
| body: expect.not.stringContaining('evaluation_environments'), | ||
| }) | ||
| ) | ||
| | ||
| await posthogWithoutEnvs.shutdown() | ||
| }) | ||
| | ||
| it('should not send evaluation environments when configured as empty array', async () => { | ||
| mockedFetch.mockImplementation( | ||
| apiImplementation({ | ||
| decideFlags: { 'test-flag': true }, | ||
| flagsPayloads: {}, | ||
| }) | ||
| ) | ||
| | ||
| const posthogWithEmptyEnvs = new PostHog('TEST_API_KEY', { | ||
| host: 'http://example.com', | ||
| evaluationEnvironments: [], | ||
| ...posthogImmediateResolveOptions, | ||
| }) | ||
| | ||
| await posthogWithEmptyEnvs.getAllFlags('some-distinct-id') | ||
| | ||
| expect(mockedFetch).toHaveBeenCalledWith( | ||
| 'http://example.com/flags/?v=2&config=true', | ||
| expect.objectContaining({ | ||
| method: 'POST', | ||
| body: expect.not.stringContaining('evaluation_environments'), | ||
| }) | ||
| ) | ||
| | ||
| await posthogWithEmptyEnvs.shutdown() | ||
| }) | ||
| }) | ||
| | ||
| describe('getRemoteConfigPayload', () => { | ||
| let requestRemoteConfigPayloadSpy: jest.SpyInstance | ||
| | ||
| | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| | @@ -7,6 +7,61 @@ Linking.getInitialURL = jest.fn(() => Promise.resolve(null)) | |
| AppState.addEventListener = jest.fn() | ||
| | ||
| describe('PostHog React Native', () => { | ||
| describe('evaluation environments', () => { | ||
| it('should send evaluation environments when configured', async () => { | ||
| posthog = new PostHog('test-token', { | ||
| evaluationEnvironments: ['production', 'mobile'], | ||
| flushInterval: 0, | ||
| }) | ||
| await posthog.ready() | ||
| | ||
| await posthog.reloadFeatureFlagsAsync() | ||
| | ||
| expect((globalThis as any).window.fetch).toHaveBeenCalledWith( | ||
| expect.stringContaining('/flags/?v=2&config=true'), | ||
| expect.objectContaining({ | ||
| method: 'POST', | ||
| body: expect.stringContaining('"evaluation_environments":["production","mobile"]'), | ||
| Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: The test uses string matching on the request body, which could be brittle if the JSON serialization order changes. Consider using JSON.parse() to validate the presence of the field more reliably. Prompt To Fix With AIThis is a comment left during a code review. Path: packages/react-native/test/posthog.spec.ts Line: 24:24 Comment: **style:** The test uses string matching on the request body, which could be brittle if the JSON serialization order changes. Consider using JSON.parse() to validate the presence of the field more reliably. How can I resolve this? If you propose a fix, please make it concise. | ||
| }) | ||
| ) | ||
| }) | ||
| | ||
| it('should not send evaluation environments when not configured', async () => { | ||
| posthog = new PostHog('test-token', { | ||
| flushInterval: 0, | ||
| }) | ||
| await posthog.ready() | ||
| | ||
| await posthog.reloadFeatureFlagsAsync() | ||
| | ||
| expect((globalThis as any).window.fetch).toHaveBeenCalledWith( | ||
| expect.stringContaining('/flags/?v=2&config=true'), | ||
| expect.objectContaining({ | ||
| method: 'POST', | ||
| body: expect.not.stringContaining('evaluation_environments'), | ||
| }) | ||
| ) | ||
| }) | ||
| | ||
| it('should not send evaluation environments when configured as empty array', async () => { | ||
| posthog = new PostHog('test-token', { | ||
| evaluationEnvironments: [], | ||
| flushInterval: 0, | ||
| }) | ||
| await posthog.ready() | ||
| | ||
| await posthog.reloadFeatureFlagsAsync() | ||
| | ||
| expect((globalThis as any).window.fetch).toHaveBeenCalledWith( | ||
| expect.stringContaining('/flags/?v=2&config=true'), | ||
| expect.objectContaining({ | ||
| method: 'POST', | ||
| body: expect.not.stringContaining('evaluation_environments'), | ||
| }) | ||
| ) | ||
| }) | ||
| }) | ||
| | ||
| let mockStorage: PostHogCustomStorage | ||
| let cache: any = {} | ||
| | ||
| | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: Missing property declaration for
evaluationEnvironments. This will cause a TypeScript compilation error since the property is not declared in the class.Prompt To Fix With AI