Skip to content

Commit

Permalink
fix(storage): Error comparing storage instances for change listener
Browse files Browse the repository at this point in the history
The old switch statement was starting to break, and couldn't consistently tell which storage was in use.

Instead, use the per-storage listeners and we don't have to worry about the storage name.
  • Loading branch information
aklinker1 committed Mar 4, 2023
1 parent 57b76b0 commit aa0edd0
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 11 deletions.
15 changes: 5 additions & 10 deletions packages/storage/src/defineExtensionStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,17 @@ interface RegisteredChangeListener<TSchema extends AnySchema> {
export function defineExtensionStorage<TSchema extends AnySchema = AnySchema>(
storage: Storage.StorageArea,
): ExtensionStorage<TSchema> {
const area = getStorageName(storage);

/**
* The singleton callback added and removed from the `browser.storage.onChanged` event. It calls
* all the listeners added to this storage instance.
*/
const onStorageChanged = async (
changes: Record<string, Storage.StorageChange>,
changedArea: string,
) => {
if (area !== changedArea) return;

const onStorageChanged = async (changes: browser.Storage.StorageAreaOnChangedChangesType) => {
const work = listeners.map(({ key, cb }) => {
if (!(key in changes)) return;

const { newValue, oldValue } = changes[key as string];
const { newValue, oldValue } = changes[
key as string
] as browser.Storage.StorageAreaOnChangedChangesType;
if (newValue === oldValue) return;

return cb(newValue, oldValue);
Expand All @@ -47,7 +42,7 @@ export function defineExtensionStorage<TSchema extends AnySchema = AnySchema>(
*/
function addListener(listener: RegisteredChangeListener<TSchema>) {
if (listeners.length === 0) {
browser.storage.onChanged.addListener(onStorageChanged);
storage.onChanged.addListener(onStorageChanged);
}

listeners.push(listener);
Expand Down
2 changes: 1 addition & 1 deletion packages/storage/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ describe('Storage Wrappers', () => {
describe('onChange', () => {
it('should only add a single onChange listener using the browser.storage.onChanged API', async () => {
const fn = vi.fn();
const addListenerSpy = vi.spyOn(fakeBrowser.storage.onChanged, 'addListener');
const addListenerSpy = vi.spyOn(fakeBrowser.storage.local.onChanged, 'addListener');

storage.onChange('key1', fn);
expect(addListenerSpy).toBeCalledTimes(1);
Expand Down

0 comments on commit aa0edd0

Please sign in to comment.