feat: delta rework #9133

sjaanus · 2025-01-22T09:11:50Z

We are changing how the Delta API works, as discussed:

We have removed the updated and removed arrays and now keep everything in the events array.
We decided to keep the hydration cache separate from the events array internally. Since the hydration cache has a special structure and may contain not just one feature but potentially 1,000 features, it behaved differently, requiring a lot of special logic to handle it.
Implemented nameprefix filtering, which we were missing before.

Things still to implement:

Segment hydration and updates to it.

vercel · 2025-01-22T09:11:55Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
unleash-monorepo-frontend	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 22, 2025 10:41am

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
unleash-docs	⬜️ Ignored (Inspect)	Visit Preview		Jan 22, 2025 10:41am

github-actions · 2025-01-22T09:12:02Z

@sjaanus, core features have been modified in this pull request. Please review carefully!

github-actions · 2025-01-22T09:12:10Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

Package	Version	Score	Details

Scanned Files

FredrikOseberg

Some feedback we'll need to address before merging. I might just be missing context but let's discuss it before we go ahead.

FredrikOseberg · 2025-01-22T11:12:32Z

src/lib/features/client-feature-toggles/client-feature-toggle.controller.ts

+                if (delta?.events[0].type === 'hydration') {
+                    const hydrationEvent: DeltaHydrationEvent =
+                        delta?.events[0];
+                    const sortedNewToggles = hydrationEvent.features.sort(
+                        (a, b) => a.name.localeCompare(b.name),
+                    );

-                if (
-                    !this.deepEqualIgnoreOrder(sortedToggles, sortedNewToggles)
-                ) {
-                    this.logger.warn(
-                        `old features and new features are different. Old count ${
-                            features.length
-                        }, new count ${delta?.updated.length}, query ${JSON.stringify(query)},
+                    if (
+                        !this.deepEqualIgnoreOrder(
+                            sortedToggles,
+                            sortedNewToggles,
+                        )
+                    ) {
+                        this.logger.warn(
+                            `old features and new features are different. Old count ${
+                                features.length
+                            }, new count ${hydrationEvent.features.length}, query ${JSON.stringify(query)},
                        diff ${JSON.stringify(
                            diff(sortedToggles, sortedNewToggles),
                        )}`,
+                        );
+                    }
+                } else {
+                    this.logger.warn(
+                        `Delta diff should have only hydration event, query ${JSON.stringify(query)}`,


Is this the correct implementation of the diff checker? If we only compare on hydration event, how do we know that the deltas are producing the correct result?

Well, every time you add new events to cache, they will instantly applied to hydration event.
We only take features from database at first, but after that we will start manipulation the hydration event every second by applying the deltas.

FredrikOseberg · 2025-01-22T11:14:29Z

src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta-types.ts

@@ -0,0 +1,63 @@
+import type { ClientFeatureSchema } from '../../../openapi';


Love this file. Very clean.

FredrikOseberg · 2025-01-22T11:22:02Z

src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta.ts

+            (isDeltaFeatureUpdatedEvent(revision) &&
+                revision.feature.name.startsWith(namePrefix)) ||
+            (isDeltaFeatureRemovedEvent(revision) &&
+                revision.featureName.startsWith(namePrefix))


Why are these different?

Feature updated has complex object of feature, with all the description, strategies, constraints. Removed has just string value.

FredrikOseberg · 2025-01-22T11:22:38Z

src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta.ts

+        return (
+            startsWithPrefix(revision) && (allProjects || isInProject(revision))
+        );


We always filter with namePrefix? What if nameprefix is empty?

https://github.com/Unleash/unleash/pull/9133/files#diff-117921481602afc7d7b53fef3f43e7c22c8e08d938f28512444b1021bdcb86c5R145

Normalizes it.

FredrikOseberg · 2025-01-22T11:24:15Z

src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta.ts

        if (requiredRevisionId >= this.currentRevisionId) {
            return undefined;
        }
+        if (requiredRevisionId === 0) {


Might be missing some context, but if this is the revisionId coming from the SDK, couldn't it possibly be undefined? Why a strict check on 0?

https://github.com/Unleash/unleash/pull/9133/files#diff-117921481602afc7d7b53fef3f43e7c22c8e08d938f28512444b1021bdcb86c5R145 normalizes it.

FredrikOseberg

LGTM

sjaanus added 5 commits January 22, 2025 09:25

feat: delta rework

6909399

feat: delta rework

ff9e27a

feat: delta rework

dfe70f6

feat: delta rework

47d9f4e

feat: delta rework

7a8bc05

github-actions bot requested review from chriswk, FredrikOseberg, gastonfournier and ivarconr January 22, 2025 09:12

github-actions bot assigned sjaanus Jan 22, 2025

feat: delta rework

f8906aa

vercel bot deployed to Preview – unleash-monorepo-frontend January 22, 2025 09:27 View deployment

feat: delta rework

af54940

vercel bot deployed to Preview – unleash-monorepo-frontend January 22, 2025 09:32 View deployment

feat: delta rework

1588b98

vercel bot deployed to Preview – unleash-monorepo-frontend January 22, 2025 09:40 View deployment

feat: delta rework

5b5d231

vercel bot deployed to Preview – unleash-monorepo-frontend January 22, 2025 09:43 View deployment

feat: delta rework

a034201

vercel bot deployed to Preview – unleash-monorepo-frontend January 22, 2025 09:48 View deployment

feat: delta rework

93c76a8

vercel bot deployed to Preview – unleash-monorepo-frontend January 22, 2025 10:16 View deployment

feat: delta rework

71b2874

vercel bot deployed to Preview – unleash-monorepo-frontend January 22, 2025 10:21 View deployment

feat: delta rework

1676063

vercel bot deployed to Preview – unleash-monorepo-frontend January 22, 2025 10:26 View deployment

feat: delta rework

30f3c4e

vercel bot deployed to Preview – unleash-monorepo-frontend January 22, 2025 10:40 View deployment

FredrikOseberg requested changes Jan 22, 2025

View reviewed changes

FredrikOseberg approved these changes Jan 22, 2025

View reviewed changes

sjaanus merged commit 280710f into main Jan 22, 2025
12 checks passed

sjaanus deleted the delta-rework branch January 22, 2025 11:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: delta rework #9133

feat: delta rework #9133

sjaanus commented Jan 22, 2025 •

edited

Loading

vercel bot commented Jan 22, 2025 •

edited

Loading

github-actions bot commented Jan 22, 2025

github-actions bot commented Jan 22, 2025 •

edited

Loading

FredrikOseberg left a comment

FredrikOseberg Jan 22, 2025

sjaanus Jan 22, 2025 •

edited

Loading

FredrikOseberg Jan 22, 2025

FredrikOseberg Jan 22, 2025

sjaanus Jan 22, 2025

FredrikOseberg Jan 22, 2025

sjaanus Jan 22, 2025

FredrikOseberg Jan 22, 2025

sjaanus Jan 22, 2025

FredrikOseberg left a comment

		@@ -0,0 +1,63 @@
		import type { ClientFeatureSchema } from '../../../openapi';

feat: delta rework #9133

feat: delta rework #9133

Conversation

sjaanus commented Jan 22, 2025 • edited Loading

vercel bot commented Jan 22, 2025 • edited Loading

github-actions bot commented Jan 22, 2025

github-actions bot commented Jan 22, 2025 • edited Loading

Dependency Review

OpenSSF Scorecard

Scanned Files

FredrikOseberg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sjaanus Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FredrikOseberg left a comment

Choose a reason for hiding this comment

sjaanus commented Jan 22, 2025 •

edited

Loading

vercel bot commented Jan 22, 2025 •

edited

Loading

github-actions bot commented Jan 22, 2025 •

edited

Loading

sjaanus Jan 22, 2025 •

edited

Loading