Skip to content

Chore: remove dead code in CommonDB #18107

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.

Sign up for GitHub

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
Lemonexe merged 2 commits into from
Apr 3, 2025
Merged

Chore: remove dead code in CommonDB #18107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7576bd5
chore(suite): remove SuiteStorage BroadcastChannel
Lemonexe Apr 2, 2025
2bd35c9
chore(suite): simplify isDBAvailable
Lemonexe Apr 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions packages/suite-storage/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@
"react-native": "./src/native/index.ts",
"browser": "./src/web/index.ts",
"dependencies": {
"@trezor/env-utils": "workspace:*",
"@trezor/eslint": "workspace:*",
"@trezor/utils": "workspace:*",
"broadcast-channel": "^7.0.0",
Copy link
Copy Markdown
Contributor Author

@Lemonexe Lemonexe Apr 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • This lib is what led me to find this obsolete code at all (I was about to bump it)
  • It' super weird because BroadcastChannel is natively offerred by browsers, and we rely on the native implementation at multiple places!
  • only here we use this polyfill lib for old browsers – meaning Chrome & Firefox < 2016 & Safari < 2022, lol

"idb": "^8.0.2"
}
}
43 changes: 7 additions & 36 deletions packages/suite-storage/src/native/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import {
StoreValue,
} from 'idb';

import { StorageMessageEvent } from './types';

export type OnUpgradeFunc<TDBStructure> = (
db: IDBPDatabase<TDBStructure>,
oldVersion: number,
Expand Down Expand Up @@ -61,47 +59,21 @@ class CommonDB<TDBStructure> {
CommonDB.instance = this;
}

static isDBAvailable = (): Promise<boolean> =>
// Firefox doesn't support indexedDB while in incognito mode, but still returns valid window.indexedDB object.
// https://bugzilla.mozilla.org/show_bug.cgi?id=781982
// so we need to try accessing the IDB. try/catch around idb.open() does not catch the error (bug in idb?), that's why we use callbacks.
// this solution calls callback function from within onerror/onsuccess event handlers.
// For other browsers checking the window.indexedDB should be enough.
Promise.resolve(false);
// const isFirefox = navigator && navigator.userAgent.toLowerCase().indexOf('firefox') > -1;
// return new Promise((resolve, _reject) => {
// if (isFirefox) {
// const r = indexedDB.open('test');
// r.onerror = () => resolve(false);
// r.onsuccess = () => resolve(true);
// } else {
// // @ts-ignore
// const idbAvailable = !!indexedDB || !!window.indexedDB || !!global.indexedDB;
// if (idbAvailable) {
// resolve(true);
// } else {
// resolve(false);
// }
// }
// });

isSupported = (): Promise<boolean> => {
static isDBAvailable = () => false;

isSupported = () => {
this.supported = false;

return Promise.resolve(false);
return false;
};

isAccessible = async () => {
const isSupported = await this.isSupported();
isAccessible = (): Promise<boolean> => {
const isSupported = this.isSupported();

// if the instance is blocking db upgrade, db connection will be closed
return isSupported && !this.blocking && !this.blocked;
return Promise.resolve(isSupported && !this.blocking && !this.blocked);
};

notify = (_store: StoreNames<TDBStructure>, _keys: any[]) => {};

onChange = (_handler: (event: StorageMessageEvent<TDBStructure>) => any) => {};

getDB = (): Promise<IDBPDatabase<TDBStructure>> =>
// @ts-expect-error
Promise.resolve();
Expand Down Expand Up @@ -205,4 +177,3 @@ class CommonDB<TDBStructure> {
}

export default CommonDB;
export type { StorageUpdateMessage } from './types';
11 changes: 0 additions & 11 deletions packages/suite-storage/src/native/types/index.ts
View file
Open in desktop

This file was deleted.

66 changes: 10 additions & 56 deletions packages/suite-storage/src/web/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { BroadcastChannel } from 'broadcast-channel';
import {
IDBPDatabase,
IDBPTransaction,
Expand All @@ -12,11 +11,8 @@ import {
unwrap,
} from 'idb';

import { isFirefox } from '@trezor/env-utils';
import { createLazy } from '@trezor/utils';

import { StorageMessageEvent } from './types';

export type OnUpgradeFunc<TDBStructure> = (
db: IDBPDatabase<TDBStructure>,
oldVersion: number,
Expand All @@ -28,7 +24,6 @@ class CommonDB<TDBStructure> {
private static instance: CommonDB<any>;
dbName!: string;
version!: number;
broadcastChannel!: BroadcastChannel;
supported: boolean | undefined;
blocking = false;
blocked = false;
Expand Down Expand Up @@ -67,39 +62,14 @@ class CommonDB<TDBStructure> {

this.isSupported();

// create global instance of broadcast channel
this.broadcastChannel = new BroadcastChannel('storageChangeEvent');

CommonDB.instance = this;
}

static isDBAvailable = () =>
// Firefox doesn't support indexedDB while in incognito mode, but still returns valid window.indexedDB object.
// https://bugzilla.mozilla.org/show_bug.cgi?id=781982
// so we need to try accessing the IDB. try/catch around idb.open() does not catch the error (bug in idb?), that's why we use callbacks.
// this solution calls callback function from within onerror/onsuccess event handlers.
// For other browsers checking the window.indexedDB should be enough.
new Promise<boolean>(resolve => {
if (isFirefox()) {
const r = indexedDB.open('test');
r.onerror = () => resolve(false);
r.onsuccess = () => {
indexedDB.deleteDatabase('test');
resolve(true);
};
Copy link
Copy Markdown
Contributor Author

@Lemonexe Lemonexe Apr 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This workaround is not needed anymore. The bug linked in comment was marked resolved 5 yrs ago.
I also tested it myself in Firefox Private window at any https page, not about:blank or file:/// etc.:

const r = indexedDB.open('test');
r.onerror = () => console.log('NOT SUPPORTED');
r.onsuccess = () => {indexedDB.deleteDatabase('test'); console.log('WORKS'); };

} else {
const idbAvailable = !!indexedDB || !!window.indexedDB || !!global.indexedDB;
if (idbAvailable) {
resolve(true);
} else {
resolve(false);
}
}
});
static isDBAvailable = () => !!indexedDB || !!window.indexedDB || !!global.indexedDB;

isSupported = async () => {
isSupported = () => {
if (this.supported === undefined) {
const isAvailable = await CommonDB.isDBAvailable();
const isAvailable = CommonDB.isDBAvailable();
this.supported = isAvailable;
if (!isAvailable) {
console.warn("Couldn't get an access to IndexedDB.");
Expand All @@ -109,22 +79,11 @@ class CommonDB<TDBStructure> {
return this.supported;
};

isAccessible = async () => {
const isSupported = await this.isSupported();
isAccessible = (): Promise<boolean> => {
const isSupported = this.isSupported();

// if the instance is blocking db upgrade, db connection will be closed
return isSupported && !this.blocking && !this.blocked;
};

notify = (store: StoreNames<TDBStructure>, keys: any[]) => {
// sends the message containing store, keys which were updated to other tabs/windows
const message = { store, keys };
this.broadcastChannel.postMessage(message);
};

onChange = (handler: (event: StorageMessageEvent<TDBStructure>) => any) => {
// listens to the channel. On receiving a message triggers the handler func
this.broadcastChannel.onmessage = handler;
return Promise.resolve(isSupported && !this.blocking && !this.blocked);
Copy link
Copy Markdown
Contributor Author

@Lemonexe Lemonexe Apr 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I thought that removing the FF workaround can reduce this whole chain of function from async to sync.
Yes it can, but it's not practical!
In storageActions there are 37 occurrences of await db.isAccessible(). These functions have to consistently return Promise, but without await you'd have to remove async, and it would be a lot of boilerplate code to Promise.resolve() everything to ensure they always return a Promise 🤦

};

closeAfterTimeout = (timeout = 1000) => {
Expand Down Expand Up @@ -206,7 +165,6 @@ class CommonDB<TDBStructure> {
reject(req.error);
};
req.onsuccess = _event => {
this.notify(store, [req.result]);
resolve(req.result);
};
} catch (error) {
Expand All @@ -231,7 +189,7 @@ class CommonDB<TDBStructure> {
const tx = db.transaction(store, 'readwrite');

const keys: StoreKey<TDBStructure, StoreNames<TDBStructure>>[] = [];
const promises = Promise.all(
const promisesOfItems: Promise<void[]> = Promise.all(
items
.map(item => {
if (upsert) {
Expand All @@ -245,11 +203,10 @@ class CommonDB<TDBStructure> {
});
})
.concat(tx.done),
).then(_ => {
this.notify(store, keys);
});
);
const aggregatedPromise: Promise<void> = promisesOfItems.then(() => {});
Copy link
Copy Markdown
Contributor Author

@Lemonexe Lemonexe Apr 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fn is expected to return Promise<void>, not Promise<void[]> (would fail downstream in suite).
Naming the function makes this clear without code commentary


return promises;
return aggregatedPromise;
};

getItemByPK = async <
Expand Down Expand Up @@ -300,7 +257,6 @@ class CommonDB<TDBStructure> {
const result = await index.get(key);
if (result) {
Object.assign(result, updateObject);
this.notify(store, [result]);

return tx.store.put(result);
}
Expand Down Expand Up @@ -339,7 +295,6 @@ class CommonDB<TDBStructure> {
let cursor = await txIdIndex.openCursor(IDBKeyRange.only(key));
while (cursor) {
cursor.delete();
this.notify(store, cursor.value ? [cursor.value] : []);

cursor = await cursor.continue();
}
Expand Down Expand Up @@ -471,4 +426,3 @@ class CommonDB<TDBStructure> {
}

export default CommonDB;
export type { StorageUpdateMessage } from './types';
11 changes: 0 additions & 11 deletions packages/suite-storage/src/web/types/index.ts
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion packages/suite-storage/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"compilerOptions": { "outDir": "./libDev" },
"include": [".", "**/*.json"],
"references": [
{ "path": "../env-utils" },
{ "path": "../eslint" },
{ "path": "../utils" }
]
Expand Down
3 changes: 0 additions & 3 deletions packages/suite/src/storage/definitions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import type {
RatesByTimestamps,
WalletSettings,
} from '@suite-common/wallet-types';
import type { StorageUpdateMessage } from '@trezor/suite-storage';

import type { SuiteState } from 'src/reducers/suite/suiteReducer';
import type { MetadataState } from 'src/types/suite/metadata';
Expand Down Expand Up @@ -146,5 +145,3 @@ export interface SuiteDBSchema extends DBSchema {
};
};
}

export type SuiteStorageUpdateMessage = StorageUpdateMessage<SuiteDBSchema>;
1 change: 0 additions & 1 deletion scripts/list-outdated-dependencies/foundation-dependencies.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ express

# STORAGE
idb
broadcast-channel

# ESLINT
@eslint/js
Expand Down
37 changes: 0 additions & 37 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1622,15 +1622,6 @@ __metadata:
languageName: node
linkType: hard

"@babel/runtime@npm:7.23.4":
version: 7.23.4
resolution: "@babel/runtime@npm:7.23.4"
dependencies:
regenerator-runtime: "npm:^0.14.0"
checksum: 10/6ef4f6dcc4ec4d74cb9f6c26a26e92d016b36debd167be48cae293fbd990b3157fb1d8d21c531285da15a5bda9ccb23e651b56234941e03d91c8af69d4c593a9
languageName: node
linkType: hard

"@babel/runtime@npm:^7.0.0, @babel/runtime@npm:^7.1.2, @babel/runtime@npm:^7.12.0, @babel/runtime@npm:^7.12.13, @babel/runtime@npm:^7.12.5, @babel/runtime@npm:^7.17.8, @babel/runtime@npm:^7.18.3, @babel/runtime@npm:^7.18.6, @babel/runtime@npm:^7.20.0, @babel/runtime@npm:^7.21.0, @babel/runtime@npm:^7.23.2, @babel/runtime@npm:^7.23.5, @babel/runtime@npm:^7.23.8, @babel/runtime@npm:^7.25.0, @babel/runtime@npm:^7.26.0, @babel/runtime@npm:^7.27.0, @babel/runtime@npm:^7.5.5, @babel/runtime@npm:^7.8.4, @babel/runtime@npm:^7.8.7, @babel/runtime@npm:^7.9.2":
version: 7.27.0
resolution: "@babel/runtime@npm:7.27.0"
Expand Down Expand Up @@ -12140,10 +12131,8 @@ __metadata:
version: 0.0.0-use.local
resolution: "@trezor/suite-storage@workspace:packages/suite-storage"
dependencies:
"@trezor/env-utils": "workspace:*"
"@trezor/eslint": "workspace:*"
"@trezor/utils": "workspace:*"
broadcast-channel: "npm:^7.0.0"
idb: "npm:^8.0.2"
languageName: unknown
linkType: soft
Expand Down Expand Up @@ -16460,18 +16449,6 @@ __metadata:
languageName: node
linkType: hard

"broadcast-channel@npm:^7.0.0":
version: 7.0.0
resolution: "broadcast-channel@npm:7.0.0"
dependencies:
"@babel/runtime": "npm:7.23.4"
oblivious-set: "npm:1.4.0"
p-queue: "npm:6.6.2"
unload: "npm:2.4.1"
checksum: 10/a7cafdd54df4cafc5c8627205001d0377e77afd7dfb8ae2969f779b1a0ae2085413d021db0ab53ec74c5a43c634687289aafa8f39d8dedfbf0b0324fc670ab18
languageName: node
linkType: hard

"brorand@npm:^1.0.1, brorand@npm:^1.1.0":
version: 1.1.0
resolution: "brorand@npm:1.1.0"
Expand Down Expand Up @@ -32448,13 +32425,6 @@ __metadata:
languageName: node
linkType: hard

"oblivious-set@npm:1.4.0":
version: 1.4.0
resolution: "oblivious-set@npm:1.4.0"
checksum: 10/4503772b19eda65d18afca528a35f73b68956833f88b64c528516159a190f61c5c0c5f7f33a7882aad267ecec67097c11729e497b0bbb34e093a3aa84679cc33
languageName: node
linkType: hard

"obuf@npm:^1.0.0, obuf@npm:^1.1.2, obuf@npm:~1.1.2":
version: 1.1.2
resolution: "obuf@npm:1.1.2"
Expand Down Expand Up @@ -40735,13 +40705,6 @@ __metadata:
languageName: node
linkType: hard

"unload@npm:2.4.1":
version: 2.4.1
resolution: "unload@npm:2.4.1"
checksum: 10/00b1181eac776c7e3bf9ea3ff93e183a926d83ad445ff616e5b5f06c0b4abab19522cf3376a4a4161e4e293f55ab4d58782c48d00a3784c878e13eb7c69d2679
languageName: node
linkType: hard

"unpipe@npm:1.0.0, unpipe@npm:~1.0.0":
version: 1.0.0
resolution: "unpipe@npm:1.0.0"
Expand Down