-
Notifications
You must be signed in to change notification settings - Fork 894
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
Use idb 7 for products using shared idb library #6154
Conversation
🦋 Changeset detectedLatest commit: 61bfd0e The changes in this PR will be included in the next version bump. This PR includes changesets to release 28 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Size Report 1Affected Products
Test Logs |
Size Analysis Report 1This report is too large (608,208 characters) to be displayed here in a GitHub comment. Please use the below link to see the full report on Google Cloud Storage.Test Logs |
Changeset File Check ✅
|
interface InstallationsDB extends DBSchema { | ||
'firebase-installations-store': { | ||
key: string; | ||
value: InstallationEntry | undefined; |
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.
Do we prefer this style of optionals instead of value?: InstallationEntry;
?
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.
It would be better to do value?: InstallationEntry
but unfortunately we're extending a type from the external idb
library and that type doesn't allow value
to be an optional property (but does allow | undefined
):
https://github.com/jakearchibald/idb/blob/231942c8a6b425e799e22885afb88751df4432ca/src/entry.ts#L124
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.
LGTM
I am finding the same error as @edi |
Someone opened the same issue here. |
Context: Until recently, we were using the external
idb
dependency for app, installations, and messaging. (This also effectively made it a dependency of analytics, performance, and remote-config, as they depend on the installations package.) However, because we were required to support IE, we had to pin theidb
version to 3.0.2, as subsequent versions ofidb
no longer supported IE. Sinceidb
is currently on version 7+, this has caused a number of issues, most recently with lack of named exports causing problems with Angular.We made a temporary fix to replace
idb
entirely with our own stopgap code in #6061Since we are planning to drop IE support at I/O, we can now put the
idb
dependency back and bump it to 7+, which should have named exports and the Nodeexports
field and support ESM.