-
Notifications
You must be signed in to change notification settings - Fork 346
chore(clerk-js): Use local environment on outage #5420
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
95a4824
chore(clerk-js): Use local environment on outage
panteliselef dd6b7d2
use variable instead of string
panteliselef 91212b3
minor change
panteliselef c1e8697
minor change
panteliselef 124c177
fix lint
panteliselef 184ecc8
Merge branch 'refs/heads/main' into elef/sdki-955-persist-environment…
panteliselef 9bad073
update bundle watch
panteliselef 24c830d
update logic
panteliselef File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@clerk/clerk-js': patch | ||
--- | ||
|
||
Fallback to locally stored environment during an outage. |
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
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
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
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
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
This file was deleted.
Oops, something went wrong.
169 changes: 169 additions & 0 deletions
169
packages/clerk-js/src/utils/__tests__/localStorage.test.ts
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,169 @@ | ||
import { SafeLocalStorage } from '../localStorage'; | ||
|
||
describe('SafeLocalStorage', () => { | ||
let mockStorage: { [key: string]: string } = {}; | ||
|
||
beforeEach(() => { | ||
mockStorage = {}; | ||
|
||
// Create a mock implementation of localStorage | ||
const localStorageMock = { | ||
getItem: (key: string) => mockStorage[key] || null, | ||
setItem: (key: string, value: string) => { | ||
mockStorage[key] = value; | ||
}, | ||
removeItem: (key: string) => { | ||
delete mockStorage[key]; | ||
}, | ||
}; | ||
|
||
// Replace window.localStorage with our mock | ||
Object.defineProperty(window, 'localStorage', { | ||
value: localStorageMock, | ||
writable: true, | ||
}); | ||
}); | ||
|
||
afterEach(() => { | ||
mockStorage = {}; | ||
jest.restoreAllMocks(); | ||
}); | ||
|
||
describe('setItem', () => { | ||
it('stores value with clerk prefix', () => { | ||
SafeLocalStorage.setItem('test', 'value'); | ||
expect(mockStorage['__clerk_test']).toBeDefined(); | ||
const parsed = JSON.parse(mockStorage['__clerk_test']); | ||
expect(parsed.value).toBe('value'); | ||
}); | ||
|
||
it('handles localStorage errors gracefully', () => { | ||
Object.defineProperty(window, 'localStorage', { | ||
value: { | ||
setItem: () => { | ||
throw new Error('Storage full'); | ||
}, | ||
}, | ||
writable: true, | ||
}); | ||
|
||
expect(() => { | ||
SafeLocalStorage.setItem('test', 'value'); | ||
}).not.toThrow(); | ||
}); | ||
|
||
it('sets expiration when provided', () => { | ||
jest.useFakeTimers(); | ||
const now = Date.now(); | ||
SafeLocalStorage.setItem('test', 'value', 1000); | ||
|
||
const stored = JSON.parse(mockStorage['__clerk_test']); | ||
expect(stored.exp).toBe(now + 1000); | ||
jest.useRealTimers(); | ||
}); | ||
|
||
it('stores complex objects correctly', () => { | ||
const complexObject = { foo: 'bar', nested: { value: 42 } }; | ||
SafeLocalStorage.setItem('complex', complexObject); | ||
const stored = JSON.parse(mockStorage['__clerk_complex']); | ||
expect(stored.value).toEqual(complexObject); | ||
}); | ||
|
||
it('does not set expiration when not provided', () => { | ||
SafeLocalStorage.setItem('test', 'value'); | ||
const stored = JSON.parse(mockStorage['__clerk_test']); | ||
expect(stored.exp).toBeUndefined(); | ||
}); | ||
}); | ||
|
||
describe('getItem', () => { | ||
it('retrieves stored value', () => { | ||
SafeLocalStorage.setItem('test', 'value'); | ||
expect(SafeLocalStorage.getItem('test', 'default')).toBe('value'); | ||
}); | ||
|
||
it('retrieves stored value when not expired', () => { | ||
SafeLocalStorage.setItem('test', 'value', 1_000); | ||
expect(SafeLocalStorage.getItem('test', 'default')).toBe('value'); | ||
}); | ||
|
||
it('returns default value when key not found', () => { | ||
expect(SafeLocalStorage.getItem('nonexistent', 'default')).toBe('default'); | ||
}); | ||
|
||
it('handles localStorage errors by returning default value', () => { | ||
Object.defineProperty(window, 'localStorage', { | ||
value: { | ||
getItem: () => { | ||
throw new Error('Storage error'); | ||
}, | ||
}, | ||
writable: true, | ||
}); | ||
|
||
expect(SafeLocalStorage.getItem('test', 'default')).toBe('default'); | ||
}); | ||
|
||
it('returns default value and removes item when expired', () => { | ||
jest.useFakeTimers(); | ||
SafeLocalStorage.setItem('test', 'value', 1_000); | ||
|
||
// Advance time beyond expiration | ||
jest.advanceTimersByTime(1_001); | ||
|
||
expect(SafeLocalStorage.getItem('test', 'default')).toBe('default'); | ||
expect(mockStorage['__clerk_test']).toBeUndefined(); | ||
jest.useRealTimers(); | ||
}); | ||
|
||
it('handles malformed JSON data by returning default value', () => { | ||
mockStorage['__clerk_malformed'] = 'not-json-data'; | ||
expect(SafeLocalStorage.getItem('malformed', 'default')).toBe('default'); | ||
}); | ||
|
||
it('handles empty stored value by returning default', () => { | ||
mockStorage['__clerk_empty'] = JSON.stringify({ value: null }); | ||
expect(SafeLocalStorage.getItem('empty', 'default')).toBe('default'); | ||
}); | ||
|
||
it('retrieves complex objects correctly', () => { | ||
const complexObject = { foo: 'bar', nested: { value: 42 } }; | ||
SafeLocalStorage.setItem('complex', complexObject); | ||
expect(SafeLocalStorage.getItem('complex', {})).toEqual(complexObject); | ||
}); | ||
|
||
it('handles edge case with zero as stored value', () => { | ||
SafeLocalStorage.setItem('zero', 0); | ||
expect(SafeLocalStorage.getItem('zero', 1)).toBe(0); | ||
}); | ||
}); | ||
|
||
describe('removeItem', () => { | ||
it('removes item with clerk prefix', () => { | ||
SafeLocalStorage.setItem('test', 'value'); | ||
expect(mockStorage['__clerk_test']).toBeDefined(); | ||
SafeLocalStorage.removeItem('test'); | ||
expect(mockStorage['__clerk_test']).toBeUndefined(); | ||
}); | ||
|
||
it('handles localStorage errors gracefully', () => { | ||
Object.defineProperty(window, 'localStorage', { | ||
value: { | ||
removeItem: () => { | ||
throw new Error('Storage error'); | ||
}, | ||
}, | ||
writable: true, | ||
}); | ||
|
||
expect(() => { | ||
SafeLocalStorage.removeItem('test'); | ||
}).not.toThrow(); | ||
}); | ||
|
||
it('does nothing when removing non-existent item', () => { | ||
SafeLocalStorage.removeItem('nonexistent'); | ||
expect(mockStorage['__clerk_nonexistent']).toBeUndefined(); | ||
}); | ||
}); | ||
}); |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
const CLERK_PREFIX = '__clerk_'; | ||
|
||
export const CLERK_ENVIRONMENT_STORAGE_ENTRY = 'environment'; | ||
|
||
interface StorageEntry<T> { | ||
value: T; | ||
exp?: number; | ||
} | ||
|
||
const serialize = JSON.stringify; | ||
const parse = JSON.parse; | ||
|
||
/** | ||
* Safe wrapper around localStorage that automatically prefixes keys with 'clerk_' | ||
* and handles potential errors and entry expiration | ||
*/ | ||
export class SafeLocalStorage { | ||
private static _key(key: string): string { | ||
return `${CLERK_PREFIX}${key}`; | ||
} | ||
|
||
private static isExpired(entry: StorageEntry<unknown>): boolean { | ||
return !!entry.exp && Date.now() > entry.exp; | ||
} | ||
|
||
static setItem(key: string, value: unknown, expiresInMs?: number): void { | ||
try { | ||
const entry: StorageEntry<unknown> = { | ||
value, | ||
...(expiresInMs && { exp: Date.now() + expiresInMs }), | ||
}; | ||
window.localStorage.setItem(this._key(key), serialize(entry)); | ||
} catch { | ||
// noop | ||
} | ||
} | ||
|
||
static getItem<T>(key: string, defaultValue: T): T { | ||
try { | ||
const item = window.localStorage.getItem(this._key(key)); | ||
if (!item) return defaultValue; | ||
const entry = parse(item) as unknown as StorageEntry<T> | undefined | null; | ||
|
||
if (!entry) { | ||
return defaultValue; | ||
} | ||
|
||
if (this.isExpired(entry)) { | ||
this.removeItem(key); | ||
return defaultValue; | ||
} | ||
|
||
return entry?.value ?? defaultValue; | ||
} catch { | ||
return defaultValue; | ||
} | ||
} | ||
|
||
static removeItem(key: string): void { | ||
try { | ||
window.localStorage.removeItem(this._key(key)); | ||
} catch { | ||
// noop | ||
} | ||
} | ||
} |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering if this logic shouldn't be contained within the EnvironmentResource class. Seems like it could simplify the implementation overall
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a bit confused here, are you advocating for this to be included in the Environment class because currently it is not.
If that is the case, I'd suggest doing this in another follow up PR, which will improve the overall architecture which would handle this for non-standard browsers (e.g. Expo).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@panteliselef Yes, I think having this logic be encapsulated in the EnvrionmentResource class would be preferrable. No issue with doing it as a follow up