Repository navigation
feat(Engine): Enable Flag Dependency in the Node.js server-side SDK - #311
Open
linear-code[bot] wants to merge 3 commits into
Open
linear-code[bot] wants to merge 3 commits into
linear-code[bot] wants to merge 3 commits into
Conversation
Port Flag Dependency from flagsmith-engine 11.1.1. Segment conditions can read other flags' evaluated results through `$.flags.<feature>` JSONPath properties, resolved lazily and transitively. Flags caught in a dependency cycle report `ERROR; code=CIRCULAR_DEPENDENCY`. Align the rest of the engine with flagsmith-engine 11.1.1, as required by engine-test-data v3.12.1: - Flag results carry the selected multivariate `variant`. - Flags without segment overrides report the `STATIC` reason. - Segments are evaluated for contexts without an identity, so environment flags are evaluated without segments. Fix engine type generation now that the evaluation context schema references the evaluation result schema. Generated with [Linear](https://linear.app/flagsmith/agent-session/389c0494-a028-440c-93fd-4dfab403905d) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
Resolve JSONPaths made of member names only, such as `$.flags.<feature>.enabled` or `$.identity.identifier`, without a JSONPath evaluation. Other paths are still evaluated by jsonpath-plus. Reuse the segment matches memoised while resolving flag dependencies when evaluating segments, so segments read through a dependency are not evaluated twice. Memoise only once a segment condition has read a flag, so environments without flag dependencies do not pay for it. Generated with [Linear](https://linear.app/flagsmith/agent-session/389c0494-a028-440c-93fd-4dfab403905d) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
Add `npm run bench`, timing engine-test-data contexts and SDK offline evaluation of environment and identity flags, with and without flag dependencies, so the engine's performance can be compared across revisions. Generated with [Linear](https://linear.app/flagsmith/agent-session/389c0494-a028-440c-93fd-4dfab403905d) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Contributes to SDK-10
With local evaluation, segments can now depend on other flags (
$.flags.<feature>.enabled,.value,.variant), the same way as flagsmith-engine#343, which edge-api uses. To pass engine-test-data v3.12.1, the engine also needed the other changes in flagsmith-engine 11.1.1.ERROR; code=CIRCULAR_DEPENDENCY.STATICreason instead ofDEFAULT.variantthat was selected.npm run generate-engine-types, which runs in the pre-commit hook. The upstream context schema now refers to the result schema, so the script was failing onmain.npm run bench. Details below.Reviewer notes:
Flag.variantis still only set by remote evaluation. Showing it for local evaluation is left out of this PR.Review effort: 4/5
How did you test this code?
npm test: 483 tests pass.npm run test:esm-build: 473 pass, 10 skipped.tscis clean, andprettieris clean on the changed files.Performance
Baseline is c9f5df1 (
main, before this PR). Both revisions ran the same bench file against the same engine-test-data v3.12.1 checkout. Runs alternated baseline/PR for 5 rounds, and each figure below is the median of the 5 per-round means. Setup: Node 24.22, 1 vCPU sandbox.tests/sdk/data/environment.json: environment flagstests/sdk/data/environment.json: identity flagsSTATIC/DEFAULTreason needs. flagsmith-engine computes it the same way.To reproduce: