Skip to content
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

feat: change query persister to chrome storage, closes #5153 #5172

Merged
merged 1 commit into from
Apr 5, 2024
Merged

feat: change query persister to chrome storage, closes #5153 #5172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@
"@stitches/react": "1.2.8",
"@storybook/addon-styling-webpack": "1.0.0",
"@styled-system/theme-get": "5.1.2",
"@tanstack/query-sync-storage-persister": "4.35.7",
"@tanstack/query-async-storage-persister": "4.35.7",
"@tanstack/react-query": "4.35.7",
"@tanstack/react-query-devtools": "4.35.7",
"@tanstack/react-query-persist-client": "4.35.7",
Expand Down
37 changes: 18 additions & 19 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion scripts/generate-manifest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ const manifest = {
author: 'Leather Wallet, LLC',
description:
'Leather is the only Bitcoin wallet you need to tap into the emerging Bitcoin economy.',
permissions: ['contextMenus', 'storage'],
permissions: ['contextMenus', 'storage', 'unlimitedStorage'],
commands: {
_execute_browser_action: {
suggested_key: {
Expand Down
15 changes: 12 additions & 3 deletions src/app/common/persistence.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
import { createSyncStoragePersister } from '@tanstack/query-sync-storage-persister';
import { createAsyncStoragePersister } from '@tanstack/query-async-storage-persister';
import { QueryClient } from '@tanstack/react-query';
import { persistQueryClient } from '@tanstack/react-query-persist-client';

import { PERSISTENCE_CACHE_TIME } from '@shared/constants';
import { IS_TEST_ENV } from '@shared/environment';

const localStoragePersistor = createSyncStoragePersister({ storage: window.localStorage });
const storage = {
getItem: async (key: string) => {
const storageVal = await chrome.storage.local.get(key);
return storageVal[key];
},
setItem: (key: string, value: string) => chrome.storage.local.set({ [key]: value }),
removeItem: (key: string) => chrome.storage.local.remove([key]),
};

const chromeStorageLocalPersister = createAsyncStoragePersister({ storage });

export const queryClient = new QueryClient({
defaultOptions: {
Expand All @@ -19,9 +28,9 @@

export async function persistAndRenderApp(renderApp: () => void) {
if (!IS_TEST_ENV)
persistQueryClient({

Check warning on line 31 in src/app/common/persistence.ts

View workflow job for this annotation

GitHub Actions / lint-eslint 

An array of Promises may be unintentional. Consider handling the promises' fulfillment or rejection with Promise.all or similar, or explicitly marking the expression as ignored with the `void` operator
queryClient,
persister: localStoragePersistor,
persister: chromeStorageLocalPersister,
buster: VERSION,
});
renderApp();
Expand Down
12 changes: 8 additions & 4 deletions src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { useMemo } from 'react';
import { useLocation, useNavigate } from 'react-router-dom';

import { HStack } from 'leather-styles/jsx';

import { BitcoinTx } from '@shared/models/transactions/bitcoin-transaction.model';
import { RouteUrls } from '@shared/route-urls';

Expand Down Expand Up @@ -45,7 +47,7 @@
},
});

const bitcoinAddress = useCurrentAccountNativeSegwitAddressIndexZero();

Check warning on line 50 in src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx

View workflow job for this annotation

GitHub Actions / lint-eslint 

'useCurrentAccountNativeSegwitAddressIndexZero' is deprecated. Use signer.address instead
const { handleOpenBitcoinTxLink: handleOpenTxLink } = useBitcoinExplorerLink();
const analytics = useAnalytics();
const caption = useMemo(() => getBitcoinTxCaption(transaction), [transaction]);
Expand Down Expand Up @@ -74,10 +76,12 @@
isOriginator && !transaction.status.confirmed && !containsTaprootInput(transaction);

const txCaption = (
<BulletSeparator>
<Caption>{caption}</Caption>
{inscriptionData ? <Caption>{inscriptionData.mime_type}</Caption> : null}
</BulletSeparator>
<HStack gap="space.02">
<BulletSeparator>
<Caption>{caption}</Caption>
{inscriptionData ? <Caption>{inscriptionData.mime_type}</Caption> : null}
</BulletSeparator>
</HStack>
);

const title = inscriptionData ? `Ordinal inscription #${inscriptionData.number}` : 'Bitcoin';
Expand Down
2 changes: 1 addition & 1 deletion src/shared/route-urls.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export enum RouteUrls {
ViewSecretKey = '/view-secret-key',

// nested routes must have relative paths
Activity = 'activity',
Activity = '/activity',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why this change? Maybe this have unintended consequences?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is ui bug - if refresh page being on activity tab => tab is not active

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested actually, and seems all good

Copy link
Contributor

Choose a reason for hiding this comment

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

Changing this will add a warning in the console, that's why I added the comment there:
// nested routes must have relative paths

I think we should solve the refresh bug differently

Copy link
Contributor

Choose a reason for hiding this comment

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

I checked and I'm pretty sure I had to change all of these routes here to avoid a console warning but I can't see that now after this change so I think it could be OK.

In future, if you are fixing bugs you should at least have a separate commit message for that fix saying what it is and why in case we need to revisit it

Receive = 'receive',
ReceiveStx = 'receive/stx',
ReceiveBtc = 'receive/btc',
Expand Down
Loading