feat(core): queued storage (Web) for logger #12791
Conversation
HuiSF
force-pushed
the
hui/feat/core/queued-storage
branch
3 times, most recently
from
10f7253 to
912924f
Compare
packages/core/src/utils/queuedStorage/createQueuedStorage.native.ts
Outdated
Show resolved
Hide resolved
packages/core/__tests__/utils/queuedStorage/queuedStorage.test.ts
Outdated
Show resolved
Hide resolved
packages/core/__tests__/utils/queuedStorage/queuedStorage.test.ts
Outdated
Show resolved
Hide resolved
| mockGetAll.mockClear(); | ||
| }); | ||
|
|
||
| test('peek() returns specified number of queued items', async () => { |
There was a problem hiding this comment.
Nit: Looks like you're intentionally mixing it and test, would it be better to add describe instead?
There was a problem hiding this comment.
it and test are identical functionally as it is an alias of test, I think they fit in in this context to save two describe() wrappers... wdyt?
HuiSF
force-pushed
the
hui/feat/core/queued-storage
branch
from
912924f to
38e0351
Compare
HuiSF
force-pushed
the
hui/feat/core/queued-storage
branch
2 times, most recently
from
e89ef1f to
f359840
Compare
|
|
||
| const openDBPromise = new Promise<IDBDatabase>((resolve, reject) => { | ||
| const { indexedDB } = window; | ||
| const openRequest = indexedDB.open(DATABASE_NAME, 1); |
| const { indexedDB } = window; | ||
| const openRequest = indexedDB.open(DATABASE_NAME, 1); | ||
|
|
||
| openRequest.onupgradeneeded = () => { |
There was a problem hiding this comment.
Nit: some doc string around when onupgradeneeded is triggered. From untrained eye, I'd expect upgrade is not needed when indexedDB version is 1?
|
|
||
| await promisifyIDBRequest(request); | ||
|
|
||
| for (const item of request.result) { |
There was a problem hiding this comment.
nit, QQ
Are we fetching only the items recently changed and append the size or are we fetching all items in db and then re-assign the size
I'm guessing it's the former. so ideally would request.result just have a single item when we're adding
There was a problem hiding this comment.
It's all items, on app reload.
| await promisifyIDBRequest(request); | ||
|
|
||
| for (const item of request.result) { | ||
| currentBytesSize += (item as QueuedItem).bytesSize; |
There was a problem hiding this comment.
nit QQ
Would openRequest.onsuccess get called when items are deleted as well. In which case this would append that size as well
There was a problem hiding this comment.
openRequest.onsuccess is being called once when opening the db.
| @@ -0,0 +1,380 @@ | |||
| // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
There was a problem hiding this comment.
nit QQ
when adding or deleting items are we also testing if the total currentBytesSize is getting updated as expected
There was a problem hiding this comment.
👍 will add a new test for this separately.
HuiSF
force-pushed
the
hui/feat/core/queued-storage
branch
from
f359840 to
5596896
Compare
Description of changes
Add implementation of a queued storage for the upcoming logger. Backed by the IndexedDB on Web.
Issue #, if available
Description of how you validated changes
Checklist
yarn testpassesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.