From 4048064e78b09492423bd117e8fbc51b09673bde Mon Sep 17 00:00:00 2001 From: alex Date: Tue, 30 May 2023 14:06:15 +0100 Subject: [PATCH] Feature flags rework (#1474) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This diff tweaks our `debugFlags` framework to support setting different default value for different environments, makes it easier to define feature flags, and makes feature flags show up in the debug menu by default. With this change, feature flags will default to being enabled in dev and preview environments, but disabled in production. Specify a feature flag like this: ```ts const featureFlags = { myCoolNewFeature: createFeatureFlag('myCoolNewFeature') } ``` optionally, pass a second value to control its defaults: ```ts const featureFlags = { featureEnabledInProduction: createFeatureFlag('someFeature', { all: true }), customEnabled: createFeatureFlag('otherFeature', {development: true, staging: false, production: false}), } ``` In code, the value can be read using `featureFlags.myFeature.value`. Remember to wrap reading it in a reactive context! ### Change Type - [x] `patch` — Bug Fix ### Test Plan - ### Release Notes [internal only change] --- apps/examples/vite.config.ts | 3 + packages/editor/api-report.md | 29 ++- packages/editor/src/index.ts | 2 +- packages/editor/src/lib/components/Canvas.tsx | 6 +- packages/editor/src/lib/utils/debug-flags.ts | 200 +++++++++++++----- packages/ui/src/lib/components/DebugPanel.tsx | 125 +++++++---- 6 files changed, 252 insertions(+), 113 deletions(-) diff --git a/apps/examples/vite.config.ts b/apps/examples/vite.config.ts index d082b4462..fa0d92502 100644 --- a/apps/examples/vite.config.ts +++ b/apps/examples/vite.config.ts @@ -15,4 +15,7 @@ export default defineConfig({ optimizeDeps: { exclude: ['@tldraw/assets'], }, + define: { + 'process.env.TLDRAW_ENV': JSON.stringify(process.env.VERCEL_ENV ?? 'development'), + }, }) diff --git a/packages/editor/api-report.md b/packages/editor/api-report.md index d2abe959c..d8c51c5c6 100644 --- a/packages/editor/api-report.md +++ b/packages/editor/api-report.md @@ -610,19 +610,21 @@ export function dataTransferItemAsString(item: DataTransferItem): Promise; +// @internal (undocumented) +export type DebugFlag = DebugFlagDef & Atom; + // @internal (undocumented) export const debugFlags: { - preventDefaultLogging: Atom; - pointerCaptureLogging: Atom; - pointerCaptureTracking: Atom; - pointerCaptureTrackingObject: Atom, unknown>; - elementRemovalLogging: Atom; - debugSvg: Atom; - throwToBlob: Atom; - peopleMenu: Atom; - logMessages: Atom; - resetConnectionEveryPing: Atom; - debugCursors: Atom; + preventDefaultLogging: DebugFlag; + pointerCaptureLogging: DebugFlag; + pointerCaptureTracking: DebugFlag; + pointerCaptureTrackingObject: DebugFlag>; + elementRemovalLogging: DebugFlag; + debugSvg: DebugFlag; + throwToBlob: DebugFlag; + logMessages: DebugFlag; + resetConnectionEveryPing: DebugFlag; + debugCursors: DebugFlag; }; // @internal (undocumented) @@ -714,6 +716,11 @@ export interface ErrorSyncedStore { // @public (undocumented) export const EVENT_NAME_MAP: Record, keyof TLEventHandlers>; +// @internal (undocumented) +export const featureFlags: { + peopleMenu: DebugFlag; +}; + // @public export function fileToBase64(file: Blob): Promise; diff --git a/packages/editor/src/index.ts b/packages/editor/src/index.ts index 3f17c2bb6..0dd24d531 100644 --- a/packages/editor/src/index.ts +++ b/packages/editor/src/index.ts @@ -209,7 +209,7 @@ export { snapToGrid, uniqueId, } from './lib/utils/data' -export { debugFlags } from './lib/utils/debug-flags' +export { debugFlags, featureFlags, type DebugFlag } from './lib/utils/debug-flags' export { loopToHtmlElement, preventDefault, diff --git a/packages/editor/src/lib/components/Canvas.tsx b/packages/editor/src/lib/components/Canvas.tsx index 4cf53ce5d..b7facf4a3 100644 --- a/packages/editor/src/lib/components/Canvas.tsx +++ b/packages/editor/src/lib/components/Canvas.tsx @@ -429,8 +429,8 @@ const DebugSvgCopy = track(function DupSvg({ id }: { id: TLShapeId }) { ) }) -const UiLogger = () => { - const logMessages = useValue(debugFlags.logMessages) +const UiLogger = track(() => { + const logMessages = debugFlags.logMessages.value return (
@@ -445,4 +445,4 @@ const UiLogger = () => { })}
) -} +}) diff --git a/packages/editor/src/lib/utils/debug-flags.ts b/packages/editor/src/lib/utils/debug-flags.ts index 8b81d10ac..20fb9c0d5 100644 --- a/packages/editor/src/lib/utils/debug-flags.ts +++ b/packages/editor/src/lib/utils/debug-flags.ts @@ -1,29 +1,55 @@ -import { atom, Atom, react } from 'signia' +import { Atom, atom, react } from 'signia' // --- 1. DEFINE --- -// Define your debug flags here. Call `createDebugValue` with the name you want -// your value to be available as on `window` and the initial value. If you don't -// want your value to be stored in session storage, pass `false` as the 3rd arg +// +// Define your debug values and feature flags here. Use `createDebugValue` to +// create an arbitrary value with defaults for production, staging, and +// development. Use `createFeatureFlag` to create a boolean flag which will be +// `true` by default in development and staging, and `false` in production. +/** @internal */ +export const featureFlags = { + // todo: remove this. it's not used, but we only have one feature flag and i + // wanted an example :( + peopleMenu: createFeatureFlag('peopleMenu'), +} satisfies Record> /** @internal */ export const debugFlags = { - preventDefaultLogging: createDebugValue('tldrawPreventDefaultLogging', false), - pointerCaptureLogging: createDebugValue('tldrawPointerCaptureLogging', false), - pointerCaptureTracking: createDebugValue('tldrawPointerCaptureTracking', false), + // --- DEBUG VALUES --- + preventDefaultLogging: createDebugValue('preventDefaultLogging', { + defaults: { all: false }, + }), + pointerCaptureLogging: createDebugValue('pointerCaptureLogging', { + defaults: { all: false }, + }), + pointerCaptureTracking: createDebugValue('pointerCaptureTracking', { + defaults: { all: false }, + }), pointerCaptureTrackingObject: createDebugValue( - 'tldrawPointerCaptureTrackingObject', + 'pointerCaptureTrackingObject', // ideally we wouldn't store this mutable value in an atom but it's not // a big deal for debug values - new Map(), - false + { + defaults: { all: new Map() }, + shouldStoreForSession: false, + } ), - elementRemovalLogging: createDebugValue('tldrawElementRemovalLogging', false), - debugSvg: createDebugValue('tldrawDebugSvg', false), - throwToBlob: createDebugValue('tldrawThrowToBlob', false), - peopleMenu: createDebugValue('tldrawPeopleMenu', false), - logMessages: createDebugValue('tldrawUiLog', []), - resetConnectionEveryPing: createDebugValue('tldrawResetConnectionEveryPing', false), - debugCursors: createDebugValue('tldrawDebugCursors', false), + elementRemovalLogging: createDebugValue('elementRemovalLogging', { + defaults: { all: false }, + }), + debugSvg: createDebugValue('debugSvg', { + defaults: { all: false }, + }), + throwToBlob: createDebugValue('throwToBlob', { + defaults: { all: false }, + }), + logMessages: createDebugValue('uiLog', { defaults: { all: [] } }), + resetConnectionEveryPing: createDebugValue('resetConnectionEveryPing', { + defaults: { all: false }, + }), + debugCursors: createDebugValue('debugCursors', { + defaults: { all: false }, + }), } declare global { @@ -31,7 +57,6 @@ declare global { tldrawLog: (message: any) => void } } -debugFlags.logMessages.set([]) if (typeof window !== 'undefined') { window.tldrawLog = (message: any) => { @@ -40,11 +65,12 @@ if (typeof window !== 'undefined') { } // --- 2. USE --- -// In normal code, read from debug flags directly by calling .get() on them: -// if (debugFlags.preventDefaultLogging.get()) { ... } +// In normal code, read from debug flags directly by calling .value on them: +// if (debugFlags.preventDefaultLogging.value) { ... } // -// In react, wrap your reads in `useDerivedValue` so they react to changes: -// const shouldLog = useDerivedValue(() => debugFlags.preventDefaultLogging.get()) +// In react, wrap your reads in `useValue` (or your component in `track`) +// so they react to changes: +// const shouldLog = useValue(debugFlags.preventDefaultLogging) // --- 3. GET FUNKY --- // If you need to do fun stuff like monkey-patching in response to flag changes, @@ -67,46 +93,118 @@ if (typeof Element !== 'undefined') { // --- IMPLEMENTATION --- // you probably don't need to read this if you're just using the debug values system -function createDebugValue(name: string, initialValue: T, shouldStore = true): Atom { - if (typeof window === 'undefined') { - return atom(`debug:${name}`, initialValue) - } +function createDebugValue( + name: string, + { + defaults, + shouldStoreForSession = true, + }: { defaults: Defaults; shouldStoreForSession?: boolean } +) { + return createDebugValueBase({ + name, + defaults, + shouldStoreForSession, + }) +} +function createFeatureFlag( + name: string, + defaults: Defaults = { all: true, production: false } +) { + return createDebugValueBase({ + name, + defaults, + shouldStoreForSession: true, + }) +} - const storedValue = shouldStore ? (getStoredInitialValue(name) as T | null) : null - const value = atom(`debug:${name}`, storedValue ?? initialValue) +function createDebugValueBase(def: DebugFlagDef): DebugFlag { + const defaultValue = getDefaultValue(def) + const storedValue = def.shouldStoreForSession + ? (getStoredInitialValue(def.name) as T | null) + : null + const valueAtom = atom(`debug:${def.name}`, storedValue ?? defaultValue) - if (shouldStore) { - react(`debug:${name}`, () => { - const currentValue = value.value - try { - if (currentValue === initialValue) { - window.sessionStorage.removeItem(`debug:${name}`) - } else { - window.sessionStorage.setItem(`debug:${name}`, JSON.stringify(currentValue)) + if (typeof window !== 'undefined') { + if (def.shouldStoreForSession) { + react(`debug:${def.name}`, () => { + const currentValue = valueAtom.value + try { + if (currentValue === defaultValue) { + window.sessionStorage.removeItem(`tldraw_debug:${def.name}`) + } else { + window.sessionStorage.setItem(`tldraw_debug:${def.name}`, JSON.stringify(currentValue)) + } + } catch { + // not a big deal } - } catch { - // not a big deal - } + }) + } + + Object.defineProperty(window, `tldraw${def.name.replace(/^[a-z]/, (l) => l.toUpperCase())}`, { + get() { + return valueAtom.value + }, + set(newValue) { + valueAtom.set(newValue) + }, + configurable: true, }) } - Object.defineProperty(window, name, { - get() { - return value.value - }, - set(newValue) { - value.set(newValue) - }, - configurable: true, - }) - - return value + return Object.assign(valueAtom, def) } function getStoredInitialValue(name: string) { try { - return JSON.parse(window.sessionStorage.getItem(`debug:${name}`) ?? 'null') + return JSON.parse(window?.sessionStorage.getItem(`tldraw_debug:${name}`) ?? 'null') } catch (err) { return null } } + +// process.env might not be defined, but we can't access it using optional +// chaining because some bundlers search for `process.env.SOMETHING` as a string +// and replace it with its value. +function readEnv(fn: () => string | undefined) { + try { + return fn() + } catch { + return null + } +} + +function getDefaultValue(def: DebugFlagDef): T { + const env = + readEnv(() => process.env.TLDRAW_ENV) ?? + readEnv(() => process.env.VERCEL_PUBLIC_TLDRAW_ENV) ?? + readEnv(() => process.env.NEXT_PUBLIC_TLDRAW_ENV) ?? + // default to production because if we don't have one of these, this is probably a library use + 'production' + + switch (env) { + case 'production': + return def.defaults.production ?? def.defaults.all + case 'preview': + case 'staging': + return def.defaults.staging ?? def.defaults.all + default: + return def.defaults.development ?? def.defaults.all + } +} + +interface Defaults { + development?: T + staging?: T + production?: T + all: T +} + +/** @internal */ +export interface DebugFlagDef { + name: string + defaults: Defaults + shouldStoreForSession: boolean +} + +/** @internal */ +export type DebugFlag = DebugFlagDef & Atom diff --git a/packages/ui/src/lib/components/DebugPanel.tsx b/packages/ui/src/lib/components/DebugPanel.tsx index 849fb34ef..a6134f6ba 100644 --- a/packages/ui/src/lib/components/DebugPanel.tsx +++ b/packages/ui/src/lib/components/DebugPanel.tsx @@ -1,4 +1,13 @@ -import { App, debugFlags, hardResetApp, TLShapePartial, uniqueId, useApp } from '@tldraw/editor' +import { + App, + DebugFlag, + debugFlags, + featureFlags, + hardResetApp, + TLShapePartial, + uniqueId, + useApp, +} from '@tldraw/editor' import * as React from 'react' import { track, useValue } from 'signia-react' import { useDialogs } from '../hooks/useDialogsProvider' @@ -64,7 +73,7 @@ const ShapeCount = function ShapeCount() { return
{count} Shapes
} -function DebugMenuContent({ +const DebugMenuContent = track(function DebugMenuContent({ renderDebugMenuItems, }: { renderDebugMenuItems: (() => React.ReactNode) | null @@ -151,36 +160,6 @@ function DebugMenuContent({ Count shapes and nodes - { - if (!debugFlags.debugCursors.value) { - debugFlags.debugCursors.set(true) - - const MAX_COLUMNS = 5 - const partials = CURSOR_NAMES.map((name, i) => { - return { - id: app.createShapeId(), - type: 'geo', - x: (i % MAX_COLUMNS) * 175, - y: Math.floor(i / MAX_COLUMNS) * 175, - props: { - text: name, - w: 150, - h: 150, - fill: 'semi', - }, - } - }) - - app.createShapes(partials) - } else { - debugFlags.debugCursors.set(false) - } - }} - > - {debugFlags.debugCursors.value ? 'Debug cursors ✓' : 'Debug cursors'} - - {(() => { if (error) throw Error('oh no!') })()} @@ -200,28 +179,80 @@ function DebugMenuContent({ - { - debugFlags.peopleMenu.set(!debugFlags.peopleMenu.value) - window.location.reload() + app.setReadOnly(r)} /> + + { + if (enabled) { + const MAX_COLUMNS = 5 + const partials = CURSOR_NAMES.map((name, i) => { + return { + id: app.createShapeId(), + type: 'geo', + x: (i % MAX_COLUMNS) * 175, + y: Math.floor(i / MAX_COLUMNS) * 175, + props: { + text: name, + w: 150, + h: 150, + fill: 'semi', + }, + } + }) + + app.createShapes(partials) + } }} - > - Toggle people menu - - { - // We need to do this manually because `updateUserDocumentSettings` does not allow toggling `isReadOnly`) - app.setReadOnly(!app.isReadOnly) - }} - > - Toggle read-only - + /> + + + {Object.values(featureFlags).map((flag) => { + return + })} {renderDebugMenuItems?.()} ) +}) + +function Toggle({ + label, + value, + onChange, +}: { + label: string + value: boolean + onChange: (newValue: boolean) => void +}) { + return ( + onChange(!value)}> + {label} + + ) } +const DebugFlagToggle = track(function DebugFlagToggle({ + flag, + onChange, +}: { + flag: DebugFlag + onChange?: (newValue: boolean) => void +}) { + return ( + `${m[0]} ${m[1].toLowerCase()}`) + .replace(/^[a-z]/, (m) => m.toUpperCase())} + value={flag.value} + onChange={(newValue) => { + flag.set(newValue) + onChange?.(newValue) + }} + /> + ) +}) + const CURSOR_NAMES = [ 'none', 'default',