-
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
Remove app-compat from peerDependencies in database-compat #6319
Conversation
🦋 Changeset detectedLatest commit: 8fc79ff The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
Changeset File Check ✅
|
Size Report 1Affected Products
Test Logs |
Size Analysis Report 1This report is too large (126,731 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 |
Looks like in |
Yes, it's used by both index.node.ts and index.ts. However I'm proposing that it's safe (if a little incorrect) to leave it out of peerDependencies because anyone using builds from index.node.ts or index.ts will be a client-side JS SDK consumer, and they'll already have it installed because npm installing Same rationale as outlined in #2082 (where we did the same thing, pre v9). I've now added an eslint-disable comment in index.node.ts (same as was already in index.ts) to avoid the eslint error from this. |
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.
Thank you, Christina! LGTM!
This prevents admin-node users from seeing a warning message during npm/yarn install:
firebase/firebase-admin-node#1656
We're taking the same approach as #2082 in that admin-node users don't need that dep at all, and client JS SDK users of the database-compat package ought to be installing the
firebase
umbrella package and should never need to worry about@firebase/app-compat
not being installed.