diff --git a/.changeset/pagelayout-resizable-persistence.md b/.changeset/pagelayout-resizable-persistence.md index 50c4762aca0..a691614b3a3 100644 --- a/.changeset/pagelayout-resizable-persistence.md +++ b/.changeset/pagelayout-resizable-persistence.md @@ -2,87 +2,80 @@ '@primer/react': minor --- -Add custom persistence options to PageLayout.Pane's `resizable` prop with controlled width support +Refine `PageLayout.Pane` resizable persistence API based on design review feedback -The `resizable` prop now accepts additional configuration options: +The `resizable` prop now has a refined API with clearer configuration types and better type safety: -- `true` - Enable resizing with default localStorage persistence (existing behavior) -- `false` - Disable resizing (existing behavior) -- `{persist: false}` - Enable resizing without any persistence (avoids hydration mismatches) -- `{persist: 'localStorage'}` - Enable resizing with explicit localStorage persistence -- `{persist: fn}` - Enable resizing with custom persistence function (e.g., server-side, IndexedDB) -- `{width: number, persist: ...}` - Controlled width mode: provide current width and persistence handler +**New Configuration Types:** -**Key Features:** +- `{persist: false}` - Enable resizing without persistence (SSR-safe, width not allowed) +- `{persist: 'localStorage', widthStorageKey: string, width: number | undefined}` - localStorage persistence with required widthStorageKey and width +- `{persist: fn, width: number | undefined}` - Custom persistence with required width -1. **Flexible persistence**: Choose between no persistence, localStorage, or custom persistence function -2. **Controlled width support**: Separate current width from default constraints using `resizable.width` -3. **SSR-friendly**: No persistence mode avoids hydration mismatches in server-rendered apps +**Key Changes:** -**New types exported:** +1. **`width` required for persistence configs**: When using localStorage or custom persistence, `width` is now required (can be `undefined` to use default). This ensures intentional state management. -- `PersistFunction` - Type for custom persistence function: `(width: number, options: SaveOptions) => void | Promise` -- `SaveOptions` - Options passed to custom persist function: `{widthStorageKey: string}` -- `PersistConfig` - Configuration object: `{width?: number, persist: false | 'localStorage' | PersistFunction}` -- `ResizableConfig` - Union type for all resizable configurations: `boolean | PersistConfig` -- `PaneWidth` - Type for preset width names: `'small' | 'medium' | 'large'` -- `PaneWidthValue` - Union type for width prop: `PaneWidth | CustomWidthOptions` +2. **`widthStorageKey` in config for localStorage**: The storage key is now part of the localStorage config object, making it explicit and avoiding accidental collisions. -**New values exported:** +3. **Custom persist functions simplified**: Custom persist functions no longer receive `widthStorageKey` - consumers manage their own storage keys and mechanisms. -- `defaultPaneWidth` - Record of preset width values: `{small: 256, medium: 296, large: 320}` +4. **Backwards compatibility maintained**: `resizable={true}` with `widthStorageKey` prop still works but is deprecated. -**Example usage:** +**Deprecations:** + +- `widthStorageKey` prop is deprecated - use `resizable={{persist: 'localStorage', widthStorageKey: '...', width}}` instead +- `resizable={true}` is implicitly deprecated - use the explicit config forms instead + +**New Exported Types:** + +- `NoPersistConfig` - Type for `{persist: false}` +- `LocalStoragePersistConfig` - Type for localStorage configuration +- `CustomPersistConfig` - Type for custom persistence configuration + +**Migration Examples:** ```tsx -// No persistence - useful for SSR to avoid hydration mismatches - +// Before: Default localStorage (deprecated) + -// Explicit localStorage persistence - +// After: Explicit localStorage with widthStorageKey in config + -// Custom persistence function - save to your own storage +// Before: Custom persist with widthStorageKey passed { - // Save to server, IndexedDB, sessionStorage, etc. myStorage.set(widthStorageKey, width) } }} + widthStorageKey="my-pane" /> -// Controlled width - separate current value from constraints -const [currentWidth, setCurrentWidth] = useState(defaultPaneWidth.medium) +// After: Custom persist manages its own storage key { - setCurrentWidth(width) - localStorage.setItem('my-pane-width', width.toString()) - } + persist: (width) => myStorage.set('my-pane', width), + width: currentWidth }} /> -// Using named size for constraints with controlled current width +// SSR-safe resizing without persistence (no change needed) + + +// Controlled width with custom persistence const [currentWidth, setCurrentWidth] = useState(defaultPaneWidth.medium) setCurrentWidth(width) + persist: (width) => setCurrentWidth(width), + width: currentWidth }} /> +``` -// Using defaultPaneWidth for initialization -import {defaultPaneWidth} from '@primer/react' +**Rationale:** -const [currentWidth, setCurrentWidth] = useState(defaultPaneWidth.large) - -``` +1. **Explicit is better**: Requiring `width` for persistence configs makes state management intentional +2. **Clearer storage key management**: localStorage configs own their storage key; custom persisters manage their own +3. **Better SSR support**: `{persist: false}` makes it clear there's no persistence to worry about +4. **Type safety**: Separate config types provide better IntelliSense and type checking diff --git a/packages/react/src/PageLayout/PageLayout.features.stories.tsx b/packages/react/src/PageLayout/PageLayout.features.stories.tsx index fc08b95b767..e361ea96bf3 100644 --- a/packages/react/src/PageLayout/PageLayout.features.stories.tsx +++ b/packages/react/src/PageLayout/PageLayout.features.stories.tsx @@ -524,3 +524,118 @@ export const ResizablePaneWithControlledWidth: StoryFn = () => { ) } ResizablePaneWithControlledWidth.storyName = 'Resizable pane with controlled width (new API)' + +/** + * Migration Examples: Old API → New API + * + * This story demonstrates migration patterns from the deprecated API to the new refined API. + */ +export const ResizablePaneMigrationExamples: StoryFn = () => { + const [currentWidth, setCurrentWidth] = React.useState(defaultPaneWidth.medium) + + const codeStyle = { + background: '#f6f8fa', + padding: '1rem', + borderRadius: '6px', + fontSize: '12px', + fontFamily: 'monospace', + whiteSpace: 'pre' as const, + display: 'block', + } + + // Code examples stored as variables to avoid HTML literal warnings + const oldApiExample1 = 'resizable={true}\nwidthStorageKey="my-pane"' + const newApiExample1 = + "resizable={{\n persist: 'localStorage',\n widthStorageKey: 'my-pane',\n width: undefined\n}}" + const newApiExample2 = 'resizable={{persist: false}}' + const oldApiExample3 = + 'resizable={{\n persist: (width, {widthStorageKey}) => {\n myStorage.set(widthStorageKey, width)\n }\n}}\nwidthStorageKey="my-key"' + const newApiExample3 = + "resizable={{\n persist: (width) => {\n myStorage.set('my-key', width)\n },\n width: currentWidth\n}}" + + return ( +
+
+ + Example 1: Default localStorage (Deprecated → New) + +
+
+ + Old API (Deprecated): + + {oldApiExample1} +
+
+ + New API: + + {newApiExample1} +
+
+
+ +
+ + Example 2: No Persistence (SSR-safe) + +
+
+ + New API (no old equivalent): + + {newApiExample2} + + Use this for SSR apps to avoid hydration mismatches + +
+
+
+ +
+ + Example 3: Custom Persistence (Simplified) + +
+
+ + Old API: + + {oldApiExample3} +
+
+ + New API (Simplified): + + {newApiExample3} +
+
+
+ +
+ + Live Example: Controlled Width with Custom Persistence + + + + + + setCurrentWidth(width), + width: currentWidth, + }} + aria-label="Side pane" + > + + + + + + +
+
+ ) +} +ResizablePaneMigrationExamples.storyName = 'Migration Examples (Old → New API)' diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index 2b41576f61e..c245c42d40c 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -606,16 +606,17 @@ export type PageLayoutPaneProps = { minWidth?: number /** * Enable resizable pane behavior. - * - `true`: Enable with default localStorage persistence + * - `true`: Enable with default localStorage persistence (DEPRECATED - use `{persist: 'localStorage', widthStorageKey, width}`) * - `false`: Disable resizing - * - `{width?: number, persist: false}`: Enable without persistence, optionally with controlled current width - * - `{width?: number, persist: 'localStorage'}`: Enable with localStorage, optionally with controlled current width - * - `{width?: number, persist: fn}`: Enable with custom persistence, optionally with controlled current width - * - * The `width` property in the config represents the current/controlled width value. - * When provided, it takes precedence over the default width from the `width` prop. + * - `{persist: false}`: Enable without persistence (SSR-safe) + * - `{persist: 'localStorage', widthStorageKey, width}`: Enable with localStorage, width required + * - `{persist: fn, width}`: Enable with custom persistence, width required */ resizable?: ResizableConfig + /** + * @deprecated Use `resizable={{persist: 'localStorage', widthStorageKey: '...', width: undefined}}` instead. + * Only used for backwards compatibility when `resizable={true}`. + */ widthStorageKey?: string padding?: keyof typeof SPACING_MAP divider?: 'none' | 'line' | ResponsiveValue<'none' | 'line', 'none' | 'line' | 'filled'> diff --git a/packages/react/src/PageLayout/index.ts b/packages/react/src/PageLayout/index.ts index a395672c243..8b4b8b5e878 100644 --- a/packages/react/src/PageLayout/index.ts +++ b/packages/react/src/PageLayout/index.ts @@ -1,8 +1,10 @@ export * from './PageLayout' export type { + NoPersistConfig, + LocalStoragePersistConfig, + CustomPersistConfig, PersistConfig, PersistFunction, - SaveOptions, ResizableConfig, PaneWidth, PaneWidthValue, diff --git a/packages/react/src/PageLayout/usePaneWidth.test.ts b/packages/react/src/PageLayout/usePaneWidth.test.ts index 9380b45c217..ff3996ac53a 100644 --- a/packages/react/src/PageLayout/usePaneWidth.test.ts +++ b/packages/react/src/PageLayout/usePaneWidth.test.ts @@ -14,8 +14,12 @@ import { isResizableEnabled, isPersistConfig, isCustomPersistFunction, - type PersistConfig, + isNoPersistConfig, + isLocalStoragePersistConfig, + isCustomPersistConfig, type PersistFunction, + type NoPersistConfig, + type CustomPersistConfig, } from './usePaneWidth' // Mock refs for hook testing @@ -182,7 +186,7 @@ describe('usePaneWidth', () => { usePaneWidth({ width: 'medium', minWidth: 256, - resizable: {width: 400, persist: false}, + resizable: {width: 400, persist: () => {}}, // Custom persist with controlled width widthStorageKey: 'test-pane', ...refs, }), @@ -200,7 +204,7 @@ describe('usePaneWidth', () => { usePaneWidth({ width: 'medium', minWidth: 256, - resizable: {width: 500, persist: 'localStorage'}, + resizable: {width: 500, persist: 'localStorage', widthStorageKey: 'test-pane'}, widthStorageKey: 'test-pane', ...refs, }), @@ -212,7 +216,7 @@ describe('usePaneWidth', () => { it('should sync when resizable.width changes', () => { const refs = createMockRefs() - type ResizableType = {width?: number; persist: false} + type ResizableType = CustomPersistConfig const {result, rerender} = renderHook( ({resizable}: {resizable: ResizableType}) => @@ -223,20 +227,20 @@ describe('usePaneWidth', () => { widthStorageKey: 'test-sync-resizable', ...refs, }), - {initialProps: {resizable: {width: 350, persist: false} as ResizableType}}, + {initialProps: {resizable: {width: 350, persist: () => {}} as ResizableType}}, ) expect(result.current.currentWidth).toBe(350) // Change resizable.width - rerender({resizable: {width: 450, persist: false}}) + rerender({resizable: {width: 450, persist: () => {}}}) expect(result.current.currentWidth).toBe(450) }) - it('should fall back to default when resizable.width is removed', () => { + it('should fall back to default when switching from controlled to uncontrolled', () => { const refs = createMockRefs() - type ResizableType = {width?: number; persist: false} + type ResizableType = CustomPersistConfig | NoPersistConfig const {result, rerender} = renderHook( ({resizable}: {resizable: ResizableType}) => @@ -247,12 +251,12 @@ describe('usePaneWidth', () => { widthStorageKey: 'test-fallback', ...refs, }), - {initialProps: {resizable: {width: 400, persist: false} as ResizableType}}, + {initialProps: {resizable: {width: 400, persist: () => {}} as ResizableType}}, ) expect(result.current.currentWidth).toBe(400) - // Remove width from resizable config + // Switch to no-persist config (uncontrolled) rerender({resizable: {persist: false}}) // Should fall back to default from width prop @@ -262,7 +266,7 @@ describe('usePaneWidth', () => { it('should not sync width prop default when resizable.width is provided', () => { const refs = createMockRefs() type WidthType = 'small' | 'medium' | 'large' - type ResizableType = {width: number; persist: false} + type ResizableType = CustomPersistConfig const {result, rerender} = renderHook( ({width, resizable}: {width: WidthType; resizable: ResizableType}) => @@ -276,7 +280,7 @@ describe('usePaneWidth', () => { { initialProps: { width: 'medium' as WidthType, - resizable: {width: 400, persist: false} as ResizableType, + resizable: {width: 400, persist: () => {}} as ResizableType, }, }, ) @@ -284,7 +288,7 @@ describe('usePaneWidth', () => { expect(result.current.currentWidth).toBe(400) // Change width prop (default changes from 296 to 320) - rerender({width: 'large', resizable: {width: 400, persist: false}}) + rerender({width: 'large', resizable: {width: 400, persist: () => {}}}) // Should NOT sync to new default because resizable.width is controlling expect(result.current.currentWidth).toBe(400) @@ -313,6 +317,27 @@ describe('usePaneWidth', () => { expect(localStorage.getItem('test-save')).toBe('450') }) + it('should use widthStorageKey prop for backwards compatibility with resizable={true}', () => { + const refs = createMockRefs() + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: true, // Legacy API + widthStorageKey: 'legacy-storage-key', + ...refs, + }), + ) + + act(() => { + result.current.saveWidth(460) + }) + + expect(result.current.currentWidth).toBe(460) + // Should use widthStorageKey from prop for backwards compatibility + expect(localStorage.getItem('legacy-storage-key')).toBe('460') + }) + it('should handle localStorage write errors gracefully', () => { const refs = createMockRefs() @@ -343,14 +368,14 @@ describe('usePaneWidth', () => { localStorage.setItem = originalSetItem }) - it('should use localStorage when {persist: "localStorage"} is provided', () => { + it('should use localStorage when {persist: "localStorage", widthStorageKey, width} is provided', () => { const refs = createMockRefs() const {result} = renderHook(() => usePaneWidth({ width: 'medium', minWidth: 256, - resizable: {persist: 'localStorage'}, - widthStorageKey: 'test-explicit-localstorage', + resizable: {persist: 'localStorage', widthStorageKey: 'test-explicit-localstorage', width: undefined}, + widthStorageKey: 'should-not-use-this', ...refs, }), ) @@ -360,12 +385,50 @@ describe('usePaneWidth', () => { }) expect(result.current.currentWidth).toBe(450) + // Should use widthStorageKey from config, not from prop expect(localStorage.getItem('test-explicit-localstorage')).toBe('450') + expect(localStorage.getItem('should-not-use-this')).toBeNull() + }) + + it('should initialize from localStorage using config widthStorageKey', () => { + localStorage.setItem('config-storage-key', '380') + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: {persist: 'localStorage', widthStorageKey: 'config-storage-key', width: undefined}, + widthStorageKey: 'prop-storage-key', + ...refs, + }), + ) + + // Should read from config widthStorageKey + expect(result.current.currentWidth).toBe(380) + }) + + it('should use controlled width over localStorage for LocalStoragePersistConfig', () => { + localStorage.setItem('test-controlled', '350') + const refs = createMockRefs() + + const {result} = renderHook(() => + usePaneWidth({ + width: 'medium', + minWidth: 256, + resizable: {persist: 'localStorage', widthStorageKey: 'test-controlled', width: 420}, + widthStorageKey: 'test-controlled', + ...refs, + }), + ) + + // Should use controlled width, not localStorage + expect(result.current.currentWidth).toBe(420) }) - it('should call custom save function with width and options', () => { + it('should call custom save function with width only', () => { const customSave = vi.fn() - const customPersister: PersistConfig = {persist: customSave} + const customPersister: CustomPersistConfig = {persist: customSave, width: undefined} const refs = createMockRefs() const {result} = renderHook(() => @@ -383,14 +446,16 @@ describe('usePaneWidth', () => { }) expect(result.current.currentWidth).toBe(450) - expect(customSave).toHaveBeenCalledWith(450, {widthStorageKey: 'my-custom-key'}) + // Should be called with only width, no options object + expect(customSave).toHaveBeenCalledWith(450) + expect(customSave).toHaveBeenCalledTimes(1) // Should NOT write to localStorage expect(localStorage.getItem('my-custom-key')).toBeNull() }) it('should handle async custom save function', async () => { const customSave = vi.fn().mockResolvedValue(undefined) - const customPersister: PersistConfig = {persist: customSave} + const customPersister: CustomPersistConfig = {persist: customSave, width: undefined} const refs = createMockRefs() const {result} = renderHook(() => @@ -408,14 +473,15 @@ describe('usePaneWidth', () => { }) expect(result.current.currentWidth).toBe(350) - expect(customSave).toHaveBeenCalledWith(350, {widthStorageKey: 'test-async'}) + // Should be called with only width, no options object + expect(customSave).toHaveBeenCalledWith(350) }) it('should handle sync errors from custom save gracefully', () => { const customSave = vi.fn(() => { throw new Error('Sync storage error') }) - const customPersister: PersistConfig = {persist: customSave} + const customPersister: CustomPersistConfig = {persist: customSave, width: undefined} const refs = createMockRefs() const {result} = renderHook(() => @@ -434,12 +500,13 @@ describe('usePaneWidth', () => { }) expect(result.current.currentWidth).toBe(450) - expect(customSave).toHaveBeenCalledWith(450, {widthStorageKey: 'test-sync-error'}) + // Should be called with only width, no options object + expect(customSave).toHaveBeenCalledWith(450) }) it('should handle async rejection from custom save gracefully', async () => { const customSave = vi.fn().mockRejectedValue(new Error('Async storage error')) - const customPersister: PersistConfig = {persist: customSave} + const customPersister: CustomPersistConfig = {persist: customSave, width: undefined} const refs = createMockRefs() const {result} = renderHook(() => @@ -459,7 +526,8 @@ describe('usePaneWidth', () => { // Wait for promise rejection to be handled await vi.waitFor(() => { - expect(customSave).toHaveBeenCalledWith(450, {widthStorageKey: 'test-async-error'}) + // Should be called with only width, no options object + expect(customSave).toHaveBeenCalledWith(450) }) expect(result.current.currentWidth).toBe(450) @@ -467,7 +535,7 @@ describe('usePaneWidth', () => { it('should not read from localStorage when custom save is provided', () => { localStorage.setItem('test-pane', '500') - const customPersister: PersistConfig = {persist: vi.fn() as PersistFunction} + const customPersister: CustomPersistConfig = {persist: vi.fn() as PersistFunction, width: undefined} const refs = createMockRefs() const {result} = renderHook(() => @@ -1197,6 +1265,56 @@ describe('constants', () => { }) describe('type guards', () => { + describe('isNoPersistConfig', () => { + it('should return true for {persist: false}', () => { + expect(isNoPersistConfig({persist: false})).toBe(true) + }) + + it('should return false for LocalStoragePersistConfig', () => { + expect(isNoPersistConfig({persist: 'localStorage', widthStorageKey: 'test', width: undefined})).toBe(false) + }) + + it('should return false for CustomPersistConfig', () => { + expect(isNoPersistConfig({persist: () => {}, width: undefined})).toBe(false) + }) + }) + + describe('isLocalStoragePersistConfig', () => { + it('should return true for {persist: "localStorage", widthStorageKey, width}', () => { + expect(isLocalStoragePersistConfig({persist: 'localStorage', widthStorageKey: 'test', width: undefined})).toBe( + true, + ) + expect(isLocalStoragePersistConfig({persist: 'localStorage', widthStorageKey: 'test', width: 300})).toBe(true) + }) + + it('should return false for NoPersistConfig', () => { + expect(isLocalStoragePersistConfig({persist: false})).toBe(false) + }) + + it('should return false for CustomPersistConfig', () => { + expect(isLocalStoragePersistConfig({persist: () => {}, width: undefined})).toBe(false) + }) + }) + + describe('isCustomPersistConfig', () => { + it('should return true for {persist: fn, width}', () => { + expect(isCustomPersistConfig({persist: () => {}, width: undefined})).toBe(true) + expect(isCustomPersistConfig({persist: () => {}, width: 300})).toBe(true) + }) + + it('should return true for async function', () => { + expect(isCustomPersistConfig({persist: async () => {}, width: undefined})).toBe(true) + }) + + it('should return false for NoPersistConfig', () => { + expect(isCustomPersistConfig({persist: false})).toBe(false) + }) + + it('should return false for LocalStoragePersistConfig', () => { + expect(isCustomPersistConfig({persist: 'localStorage', widthStorageKey: 'test', width: undefined})).toBe(false) + }) + }) + describe('isResizableEnabled', () => { it('should return true for boolean true', () => { expect(isResizableEnabled(true)).toBe(true) @@ -1211,11 +1329,11 @@ describe('type guards', () => { }) it('should return true for {persist: "localStorage"}', () => { - expect(isResizableEnabled({persist: 'localStorage'})).toBe(true) + expect(isResizableEnabled({persist: 'localStorage', widthStorageKey: 'test', width: undefined})).toBe(true) }) it('should return true for {persist: fn} (custom persistence)', () => { - expect(isResizableEnabled({persist: () => {}})).toBe(true) + expect(isResizableEnabled({persist: () => {}, width: undefined})).toBe(true) }) }) @@ -1225,11 +1343,11 @@ describe('type guards', () => { }) it('should return true for {persist: "localStorage"}', () => { - expect(isPersistConfig({persist: 'localStorage'})).toBe(true) + expect(isPersistConfig({persist: 'localStorage', widthStorageKey: 'test', width: undefined})).toBe(true) }) it('should return true for {persist: fn}', () => { - expect(isPersistConfig({persist: () => {}})).toBe(true) + expect(isPersistConfig({persist: () => {}, width: undefined})).toBe(true) }) it('should return false for boolean true', () => { diff --git a/packages/react/src/PageLayout/usePaneWidth.ts b/packages/react/src/PageLayout/usePaneWidth.ts index 392ec91e9de..dfa6d004c27 100644 --- a/packages/react/src/PageLayout/usePaneWidth.ts +++ b/packages/react/src/PageLayout/usePaneWidth.ts @@ -24,29 +24,73 @@ export type PaneWidthValue = PaneWidth | CustomWidthOptions /** * Options passed to custom persist function. + * Note: widthStorageKey is no longer passed - custom persisters manage their own storage. */ -export type SaveOptions = {widthStorageKey: string} +// eslint-disable-next-line @typescript-eslint/no-empty-object-type +export type SaveOptions = {} /** * Custom persist function type. + * Receives the width value; consumer manages their own storage key/mechanism. */ -export type PersistFunction = (width: number, options: SaveOptions) => void | Promise +export type PersistFunction = (width: number) => void | Promise /** - * Configuration object for resizable pane. - * - `width?: number` - Current/controlled width value in pixels (overrides width prop's default) - * - `persist: false` - Enable resizing without any persistence - * - `persist: 'localStorage'` - Enable resizing with localStorage persistence - * - `persist: fn` - Enable resizing with custom persistence function + * No persistence - SSR safe, uses default width from width prop. + * width is not allowed here - always uses default. */ -export type PersistConfig = { - /** Current controlled width value in pixels. When provided, this overrides the default from the width prop. */ - width?: number - persist: false | 'localStorage' | PersistFunction +export type NoPersistConfig = { + persist: false +} + +/** + * localStorage persistence - requires explicit storage key. + * width is REQUIRED (can be undefined to use default). + */ +export type LocalStoragePersistConfig = { + persist: 'localStorage' + /** Storage key for localStorage. Required to avoid key collisions. */ + widthStorageKey: string + /** Current controlled width value. Required - use undefined for "use default". */ + width: number | undefined +} + +/** + * Custom persistence - consumer handles storage. + * width is REQUIRED (can be undefined to use default). + */ +export type CustomPersistConfig = { + persist: PersistFunction + /** Current controlled width value. Required - use undefined for "use default". */ + width: number | undefined +} + +export type PersistConfig = NoPersistConfig | LocalStoragePersistConfig | CustomPersistConfig + +/** + * Type guard to check if config is NoPersistConfig + */ +export const isNoPersistConfig = (config: PersistConfig): config is NoPersistConfig => { + return config.persist === false +} + +/** + * Type guard to check if config is LocalStoragePersistConfig + */ +export const isLocalStoragePersistConfig = (config: PersistConfig): config is LocalStoragePersistConfig => { + return config.persist === 'localStorage' +} + +/** + * Type guard to check if config is CustomPersistConfig + */ +export const isCustomPersistConfig = (config: PersistConfig): config is CustomPersistConfig => { + return typeof config.persist === 'function' } /** * Type guard to check if persist value is a custom function + * @deprecated Use isCustomPersistConfig instead */ export const isCustomPersistFunction = ( persist: false | 'localStorage' | PersistFunction, @@ -56,14 +100,11 @@ export const isCustomPersistFunction = ( /** * Resizable configuration options. - * - `true`: Enable resizing with default localStorage persistence (may cause hydration mismatch) + * - `true`: Enable resizing with default localStorage persistence (DEPRECATED - use {persist: 'localStorage', widthStorageKey, width} instead) * - `false`: Disable resizing - * - `{width?: number, persist: false}`: Enable resizing without any persistence, optionally with controlled width - * - `{width?: number, persist: 'localStorage'}`: Enable resizing with localStorage persistence, optionally with controlled width - * - `{width?: number, persist: fn}`: Enable resizing with custom persistence function, optionally with controlled width - * - * The `width` property in the config object represents the current/controlled width value. - * When provided, it takes precedence over the default width from the `width` prop. + * - `{persist: false}`: Enable resizing without persistence (SSR-safe) + * - `{persist: 'localStorage', widthStorageKey, width}`: Enable with localStorage, width required + * - `{persist: fn, width}`: Enable with custom persistence, width required */ export type ResizableConfig = boolean | PersistConfig @@ -256,13 +297,23 @@ export function usePaneWidth({ // --- State --- // Current width for React renders (ARIA attributes). Updates go through saveWidth() or clamp on resize. // Priority order for initial width: - // 1. resizable.width (controlled current value) - // 2. localStorage (resizable === true only) + // 1. resizable.width (controlled current value) - for LocalStoragePersistConfig and CustomPersistConfig + // 2. localStorage (resizable === true or LocalStoragePersistConfig with undefined width) // 3. defaultWidth (from width prop) const [currentWidth, setCurrentWidth] = React.useState(() => { // Check if resizable config has a controlled width value - if (isPersistConfig(resizable) && typeof resizable.width === 'number') { - return resizable.width + if (isPersistConfig(resizable) && !isNoPersistConfig(resizable)) { + // For LocalStoragePersistConfig and CustomPersistConfig, width is required (can be undefined) + if (resizable.width !== undefined) { + return resizable.width + } + // For LocalStoragePersistConfig with width: undefined, try localStorage + if (isLocalStoragePersistConfig(resizable)) { + const storedWidth = localStoragePersister.get(resizable.widthStorageKey) + if (storedWidth !== null) { + return storedWidth + } + } } // Only try localStorage for default persister (resizable === true) // Read directly here instead of via persister to satisfy react-hooks/refs lint rule @@ -278,7 +329,7 @@ export function usePaneWidth({ // Inline state sync when width prop or resizable.width changes (avoids effect) // See: https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes const [prevDefaultWidth, setPrevDefaultWidth] = React.useState(defaultWidth) - const controlledWidth = isPersistConfig(resizable) ? resizable.width : undefined + const controlledWidth = isPersistConfig(resizable) && !isNoPersistConfig(resizable) ? resizable.width : undefined const [prevControlledWidth, setPrevControlledWidth] = React.useState(controlledWidth) // Handle controlled width changes @@ -326,12 +377,17 @@ export function usePaneWidth({ const config = resizableRef.current - // Handle localStorage persistence: resizable === true or {persist: 'localStorage'} - if (config === true || (isPersistConfig(config) && config.persist === 'localStorage')) { + // Handle localStorage persistence: resizable === true or {persist: 'localStorage', ...} + if (config === true) { + // Backwards compatibility: use widthStorageKey prop localStoragePersister.save(widthStorageKeyRef.current, value) - } else if (isPersistConfig(config) && isCustomPersistFunction(config.persist)) { + } else if (isPersistConfig(config) && isLocalStoragePersistConfig(config)) { + // Use widthStorageKey from config + localStoragePersister.save(config.widthStorageKey, value) + } else if (isPersistConfig(config) && isCustomPersistConfig(config)) { try { - const result = config.persist(value, {widthStorageKey: widthStorageKeyRef.current}) + // Custom persister - no widthStorageKey passed + const result = config.persist(value) // Handle async rejections silently if (result instanceof Promise) { // eslint-disable-next-line github/no-then