fix(features): address Wes review (Zod, route warnings, version key)
- B1: real Zod manifest validation; on parse failure log + return empty
- C1: invert preview-route gating (Pulse, projects, workflows, forum
post) — render screen and surface a sonner toast.warning via
new usePreviewFeatureWarning hook so direct links still work
- C3: derive localStorage overrides key from manifest.version
- N1: replace stale 'avoids JSON.parse' comment with accurate
reference-equality cache rationale
- N2: add cross-window 'storage' listener to mirror channel sections /
stars / mutes / thread follows pattern
- N3: explicit getServerSnapshot returning '{}' for SSR/test safety
- N4: swap 'channels' for 'doctor' in resolveEnabled test (channels
isn't in the manifest; doctor is stable)
- N5: drop screenshot-feature-flags from smoke testMatch — runs only
under playwright-screenshot.config.ts now
Signed-off-by: npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6w <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@sprout-oss.stage.blox.sqprod.co>
@@ -70,7 +70,8 @@
|
||||
"sonner": "^2.0.7",
|
||||
"tailwind-merge": "^3.5.0",
|
||||
"tiptap-markdown": "^0.9.0",
|
||||
"yaml": "^2.8.3"
|
||||
"yaml": "^2.8.3",
|
||||
"zod": "^4.4.3"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@noble/hashes": "^2.0.1",
|
||||
|
||||
@@ -35,7 +35,6 @@ export default defineConfig({
|
||||
"**/workflows.spec.ts",
|
||||
"**/identity-archive.spec.ts",
|
||||
"**/identity-archive-hide.spec.ts",
|
||||
"**/screenshot-feature-flags.ts",
|
||||
],
|
||||
use: {
|
||||
...devices["Desktop Chrome"],
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import * as React from "react";
|
||||
import { createFileRoute } from "@tanstack/react-router";
|
||||
|
||||
import { usePreviewFeatureWarning } from "@/shared/features";
|
||||
import { ViewLoadingFallback } from "@/shared/ui/ViewLoadingFallback";
|
||||
|
||||
type ForumPostRouteSearch = {
|
||||
@@ -29,6 +30,7 @@ const ChannelRouteScreen = React.lazy(async () => {
|
||||
});
|
||||
|
||||
function ForumPostRouteComponent() {
|
||||
usePreviewFeatureWarning("forum");
|
||||
const { channelId, postId } = Route.useParams();
|
||||
const search = Route.useSearch();
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import * as React from "react";
|
||||
import { createFileRoute } from "@tanstack/react-router";
|
||||
|
||||
import { usePreviewFeatureWarning } from "@/shared/features";
|
||||
import { ViewLoadingFallback } from "@/shared/ui/ViewLoadingFallback";
|
||||
|
||||
const ProjectDetailScreen = React.lazy(async () => {
|
||||
@@ -13,6 +14,7 @@ export const Route = createFileRoute("/projects/$projectId")({
|
||||
});
|
||||
|
||||
function ProjectDetailRouteComponent() {
|
||||
usePreviewFeatureWarning("projects");
|
||||
const { projectId } = Route.useParams();
|
||||
|
||||
return (
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import * as React from "react";
|
||||
import { createFileRoute } from "@tanstack/react-router";
|
||||
|
||||
import { usePreviewFeatureWarning } from "@/shared/features";
|
||||
import { ViewLoadingFallback } from "@/shared/ui/ViewLoadingFallback";
|
||||
|
||||
const ProjectsScreen = React.lazy(async () => {
|
||||
@@ -13,6 +14,7 @@ export const Route = createFileRoute("/projects")({
|
||||
});
|
||||
|
||||
function ProjectsRouteComponent() {
|
||||
usePreviewFeatureWarning("projects");
|
||||
return (
|
||||
<React.Suspense
|
||||
fallback={<ViewLoadingFallback includeHeader kind="projects" />}
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import * as React from "react";
|
||||
import { createFileRoute, Navigate } from "@tanstack/react-router";
|
||||
import { createFileRoute } from "@tanstack/react-router";
|
||||
|
||||
import { FeatureGate } from "@/shared/features";
|
||||
import { usePreviewFeatureWarning } from "@/shared/features";
|
||||
import { ViewLoadingFallback } from "@/shared/ui/ViewLoadingFallback";
|
||||
|
||||
const PulseScreen = React.lazy(async () => {
|
||||
@@ -14,13 +14,12 @@ export const Route = createFileRoute("/pulse")({
|
||||
});
|
||||
|
||||
function PulseRouteComponent() {
|
||||
usePreviewFeatureWarning("pulse");
|
||||
return (
|
||||
<FeatureGate feature="pulse" fallback={<Navigate to="/" />}>
|
||||
<React.Suspense
|
||||
fallback={<ViewLoadingFallback includeHeader kind="pulse" />}
|
||||
>
|
||||
<PulseScreen />
|
||||
</React.Suspense>
|
||||
</FeatureGate>
|
||||
<React.Suspense
|
||||
fallback={<ViewLoadingFallback includeHeader kind="pulse" />}
|
||||
>
|
||||
<PulseScreen />
|
||||
</React.Suspense>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import * as React from "react";
|
||||
import { createFileRoute } from "@tanstack/react-router";
|
||||
|
||||
import { usePreviewFeatureWarning } from "@/shared/features";
|
||||
import { ViewLoadingFallback } from "@/shared/ui/ViewLoadingFallback";
|
||||
|
||||
export const Route = createFileRoute("/workflows/$workflowId")({
|
||||
@@ -13,6 +14,7 @@ const WorkflowsRouteScreen = React.lazy(async () => {
|
||||
});
|
||||
|
||||
function WorkflowDetailRouteComponent() {
|
||||
usePreviewFeatureWarning("workflows");
|
||||
const { workflowId } = Route.useParams();
|
||||
|
||||
return (
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import * as React from "react";
|
||||
import { createFileRoute } from "@tanstack/react-router";
|
||||
|
||||
import { usePreviewFeatureWarning } from "@/shared/features";
|
||||
import { ViewLoadingFallback } from "@/shared/ui/ViewLoadingFallback";
|
||||
|
||||
export const Route = createFileRoute("/workflows")({
|
||||
@@ -13,6 +14,7 @@ const WorkflowsRouteScreen = React.lazy(async () => {
|
||||
});
|
||||
|
||||
function WorkflowsRouteComponent() {
|
||||
usePreviewFeatureWarning("workflows");
|
||||
return (
|
||||
<React.Suspense
|
||||
fallback={<ViewLoadingFallback includeHeader kind="workflows" />}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
export { FeatureGate } from "./FeatureGate";
|
||||
export { allFeatures, desktopFeatures, getFeature } from "./manifest";
|
||||
export { allFeatures, desktopFeatures, getFeature, manifest } from "./manifest";
|
||||
export { getOverrides, setOverride, clearOverride } from "./store";
|
||||
export type {
|
||||
FeatureDefinition,
|
||||
@@ -11,5 +11,6 @@ export {
|
||||
useFeatureEnabled,
|
||||
useFeatureToggle,
|
||||
useFeatureSnapshot,
|
||||
usePreviewFeatureWarning,
|
||||
resolveEnabled,
|
||||
} from "./useFeatureEnabled";
|
||||
|
||||
@@ -1,7 +1,48 @@
|
||||
import manifestJson from "@features-manifest";
|
||||
import { z } from "zod";
|
||||
import type { FeatureDefinition, FeaturesManifest } from "./types";
|
||||
|
||||
const manifest = manifestJson as FeaturesManifest;
|
||||
// ---------------------------------------------------------------------------
|
||||
// Schema — runtime-validates the bundled features.json at startup.
|
||||
//
|
||||
// On parse failure we fall back to an empty manifest and log a console warning.
|
||||
// The app keeps working; gated UI stays hidden; nothing accidentally leaks.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const FeatureTierSchema = z.enum(["stable", "preview"]);
|
||||
const FeaturePlatformSchema = z.enum(["desktop", "mobile"]);
|
||||
|
||||
const FeatureDefinitionSchema = z.object({
|
||||
id: z.string().min(1),
|
||||
name: z.string().min(1),
|
||||
description: z.string(),
|
||||
tier: FeatureTierSchema,
|
||||
platforms: z.array(FeaturePlatformSchema).optional(),
|
||||
});
|
||||
|
||||
const FeaturesManifestSchema = z.object({
|
||||
version: z.number().int().nonnegative(),
|
||||
features: z.array(FeatureDefinitionSchema),
|
||||
});
|
||||
|
||||
const EMPTY_MANIFEST: FeaturesManifest = { version: 1, features: [] };
|
||||
|
||||
function loadManifest(): FeaturesManifest {
|
||||
const result = FeaturesManifestSchema.safeParse(manifestJson);
|
||||
if (!result.success) {
|
||||
console.warn(
|
||||
"[FeatureFlags] features.json failed schema validation; falling back to empty manifest.",
|
||||
result.error.issues,
|
||||
);
|
||||
return EMPTY_MANIFEST;
|
||||
}
|
||||
return result.data;
|
||||
}
|
||||
|
||||
const manifest = loadManifest();
|
||||
|
||||
/** The validated manifest. Use `manifest.version` for cache/storage keys. */
|
||||
export { manifest };
|
||||
|
||||
/** All features defined in the manifest */
|
||||
export const allFeatures: FeatureDefinition[] = manifest.features;
|
||||
|
||||
@@ -6,11 +6,8 @@ import { resolveEnabled } from "./resolveEnabled.ts";
|
||||
describe("resolveEnabled", () => {
|
||||
describe("stable tier", () => {
|
||||
it("always returns true regardless of overrides", () => {
|
||||
assert.equal(resolveEnabled("stable", "channels", {}), true);
|
||||
assert.equal(
|
||||
resolveEnabled("stable", "channels", { channels: false }),
|
||||
true,
|
||||
);
|
||||
assert.equal(resolveEnabled("stable", "doctor", {}), true);
|
||||
assert.equal(resolveEnabled("stable", "doctor", { doctor: false }), true);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -1,11 +1,15 @@
|
||||
/**
|
||||
* Persistence layer for feature flag overrides.
|
||||
*
|
||||
* localStorage keys (versioned to match manifest):
|
||||
* sprout-feature-overrides-v1 — JSON object of { [featureId]: boolean }
|
||||
* The localStorage key is derived from `manifest.version` so a schema bump
|
||||
* naturally orphans the old key — clean reset, no migration logic.
|
||||
*
|
||||
* sprout-feature-overrides-v${manifest.version}
|
||||
* → JSON object of { [featureId]: boolean }
|
||||
*/
|
||||
import { manifest } from "./manifest";
|
||||
|
||||
const OVERRIDES_KEY = "sprout-feature-overrides-v1";
|
||||
export const OVERRIDES_KEY = `sprout-feature-overrides-v${manifest.version}`;
|
||||
|
||||
export type FeatureOverrides = Record<string, boolean>;
|
||||
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { useSyncExternalStore, useCallback } from "react";
|
||||
import { useSyncExternalStore, useCallback, useEffect } from "react";
|
||||
import { getFeature } from "./manifest";
|
||||
import { resolveEnabled } from "./resolveEnabled";
|
||||
import { getOverrides, setOverride } from "./store";
|
||||
import { getOverrides, setOverride, OVERRIDES_KEY } from "./store";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Reactive store — components re-render when overrides change
|
||||
@@ -12,7 +12,22 @@ const listeners = new Set<Listener>();
|
||||
|
||||
function subscribe(listener: Listener): () => void {
|
||||
listeners.add(listener);
|
||||
return () => listeners.delete(listener);
|
||||
|
||||
// Cross-window sync: another window writing the overrides key in
|
||||
// localStorage fires a "storage" event in this window. Mirror the
|
||||
// pattern used by useChannelSections / useChannelStars / useChannelMutes /
|
||||
// useThreadFollows.
|
||||
const handleStorage = (event: StorageEvent) => {
|
||||
if (event.key === OVERRIDES_KEY) {
|
||||
emitChange();
|
||||
}
|
||||
};
|
||||
window.addEventListener("storage", handleStorage);
|
||||
|
||||
return () => {
|
||||
listeners.delete(listener);
|
||||
window.removeEventListener("storage", handleStorage);
|
||||
};
|
||||
}
|
||||
|
||||
/** Notify all subscribers that feature state changed */
|
||||
@@ -24,7 +39,13 @@ export function emitChange(): void {
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Cached snapshot — avoids JSON.parse on every render per hook instance
|
||||
// Cached snapshot
|
||||
//
|
||||
// useSyncExternalStore requires getSnapshot to return a referentially stable
|
||||
// value when nothing has changed. Returning `JSON.stringify(getOverrides())`
|
||||
// fresh on every render would produce a new string each tick → infinite
|
||||
// re-render. We cache the serialized form and only mint a new parsed object
|
||||
// when the serialized form changes.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
let cachedRaw: string | null = null;
|
||||
@@ -39,6 +60,16 @@ function getSnapshot(): string {
|
||||
return raw;
|
||||
}
|
||||
|
||||
/**
|
||||
* Server-side snapshot for useSyncExternalStore.
|
||||
*
|
||||
* Sprout is a Tauri desktop app and does not currently SSR. Returning an
|
||||
* explicit empty-state snapshot is safer than omitting this argument: under
|
||||
* any future test harness or SSR experiment, the hook returns "no overrides"
|
||||
* instead of throwing.
|
||||
*/
|
||||
const getServerSnapshot = (): string => "{}";
|
||||
|
||||
function getParsedSnapshot(): Record<string, boolean> {
|
||||
// Ensure snapshot is fresh
|
||||
getSnapshot();
|
||||
@@ -55,7 +86,7 @@ function getParsedSnapshot(): Record<string, boolean> {
|
||||
* Use this in components that need the full state (e.g. SettingsView filtering).
|
||||
*/
|
||||
export function useFeatureSnapshot(): Record<string, boolean> {
|
||||
useSyncExternalStore(subscribe, getSnapshot, getSnapshot);
|
||||
useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot);
|
||||
return getParsedSnapshot();
|
||||
}
|
||||
|
||||
@@ -100,5 +131,37 @@ export function useFeatureToggle(
|
||||
return [enabled, toggle];
|
||||
}
|
||||
|
||||
/**
|
||||
* Fires a sonner toast.warning when a preview feature is currently disabled.
|
||||
*
|
||||
* Usage: drop in at the top of a route component to give users hitting a
|
||||
* direct link to a disabled preview feature a hint about how to surface it.
|
||||
*
|
||||
* function PulseRouteComponent() {
|
||||
* usePreviewFeatureWarning("pulse");
|
||||
* return <PulseScreen />;
|
||||
* }
|
||||
*
|
||||
* Stays a no-op for stable features and for preview features that ARE enabled.
|
||||
*/
|
||||
export function usePreviewFeatureWarning(featureId: string): void {
|
||||
const enabled = useFeatureEnabled(featureId);
|
||||
const feature = getFeature(featureId);
|
||||
|
||||
useEffect(() => {
|
||||
if (feature?.tier !== "preview" || enabled) return;
|
||||
let cancelled = false;
|
||||
void import("sonner").then(({ toast }) => {
|
||||
if (cancelled) return;
|
||||
toast.warning(
|
||||
`${feature.name} is a preview feature. Enable it in Settings → Experiments to surface it in your sidebar.`,
|
||||
);
|
||||
});
|
||||
return () => {
|
||||
cancelled = true;
|
||||
};
|
||||
}, [feature, enabled]);
|
||||
}
|
||||
|
||||
// Re-export for consumers that imported from here
|
||||
export { resolveEnabled } from "./resolveEnabled";
|
||||
|
||||
|
Before Width: | Height: | Size: 40 KiB After Width: | Height: | Size: 38 KiB |
|
Before Width: | Height: | Size: 39 KiB After Width: | Height: | Size: 38 KiB |
|
Before Width: | Height: | Size: 47 KiB After Width: | Height: | Size: 48 KiB |
|
Before Width: | Height: | Size: 56 KiB After Width: | Height: | Size: 57 KiB |
9
pnpm-lock.yaml
generated
@@ -171,6 +171,9 @@ importers:
|
||||
yaml:
|
||||
specifier: ^2.8.3
|
||||
version: 2.9.0
|
||||
zod:
|
||||
specifier: ^4.4.3
|
||||
version: 4.4.3
|
||||
devDependencies:
|
||||
'@noble/hashes':
|
||||
specifier: ^2.0.1
|
||||
@@ -1303,7 +1306,6 @@ packages:
|
||||
engines: {node: ^20.19.0 || >=22.12.0}
|
||||
cpu: [arm64]
|
||||
os: [linux]
|
||||
libc: [glibc]
|
||||
|
||||
'@rolldown/binding-linux-arm64-musl@1.0.2':
|
||||
resolution: {integrity: sha512-QVLO/czFMdoMFSqlX3bcswcJNm/23r+qoa/jgtmFc/qEp6/jXmIkDjF/XIo8dPfGaiwy1xfQn8o77L79GeXFgw==}
|
||||
@@ -1444,14 +1446,12 @@ packages:
|
||||
engines: {node: '>= 20'}
|
||||
cpu: [arm64]
|
||||
os: [linux]
|
||||
libc: [glibc]
|
||||
|
||||
'@tailwindcss/oxide-linux-arm64-musl@4.3.0':
|
||||
resolution: {integrity: sha512-Z6sukiQsngnWO+l39X4pPbiWT81IC+PLKF+PHxIlyZbGNb9MODfYlXEVlFvej5BOZInWX01kVyzeLvHsXhfczQ==}
|
||||
engines: {node: '>= 20'}
|
||||
cpu: [arm64]
|
||||
os: [linux]
|
||||
libc: [musl]
|
||||
|
||||
'@tailwindcss/oxide-linux-x64-gnu@4.3.0':
|
||||
resolution: {integrity: sha512-DRNdQRpSGzRGfARVuVkxvM8Q12nh19l4BF/G7zGA1oe+9wcC6saFBHTISrpIcKzhiXtSrlSrluCfvMuledoCTQ==}
|
||||
@@ -1608,7 +1608,6 @@ packages:
|
||||
engines: {node: '>= 10'}
|
||||
cpu: [riscv64]
|
||||
os: [linux]
|
||||
libc: [glibc]
|
||||
|
||||
'@tauri-apps/cli-linux-x64-gnu@2.11.2':
|
||||
resolution: {integrity: sha512-Ru4gwJKPG0ctVGchRGpRup4Y4lW2SSfFnrbQcyHhCliKy4g8Qz97TrUgCur4CbWyAgKxvGh3SjrkA0LDYzDGiw==}
|
||||
@@ -3027,7 +3026,7 @@ packages:
|
||||
engines: {node: '>= 0.4'}
|
||||
|
||||
wrappy@1.0.2:
|
||||
resolution: {integrity: sha512-l4Sp/DRseor9wL6EvV2+TuQn63dMkPjZ/sp9XkghTEbV9KlPS1xUsZ3u7/IQO4wxtcFB4bgpQPRcR3QCvezPcQ==}
|
||||
resolution: {integrity: sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=}
|
||||
|
||||
yallist@3.1.1:
|
||||
resolution: {integrity: sha512-a4UGQaWPH59mOXUYnAG2ewncQS4i4F43Tv3JoAM+s2VDAmS9NsK8GpDMLrCHPksFT7h3K6TOoUNn2pb7RoXx4g==}
|
||||
|
||||