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

Jump to bottom

Switch from 'grpc' to '@grpc/grpc-js' dependency. #1804

Merged
merged 2 commits into from
May 21, 2019
Merged

Switch from 'grpc' to '@grpc/grpc-js' dependency. #1804

merged 2 commits into from
May 21, 2019

Conversation

mikelehen
Copy link
Contributor

Fixes #1783, reduces package size, and should ease compatibility issues with Electron, etc.

Switch from 'grpc' to '@grpc/grpc-js' dependency.
80be07b 
Fixes #1783, reduces package size, and should ease compatibility issues with Electron, etc.
Feiyang1
Feiyang1 approved these changes May 21, 2019
View reviewed changes
Copy link
Member

@Feiyang1 Feiyang1 left a comment

Choose a reason for hiding this comment

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

👍

@mikelehen mikelehen merged commit 4771184 into master May 21, 2019
@mikelehen mikelehen deleted the mikelehen/switch-to-grpc-js branch May 21, 2019 19:04
@jeffreypriebe
Copy link

@mikelehen This seems great, but the latest release of @firebase/firestore now doesn't work on node 6 with this, given @grpc/grpc-js requirement for node v8.

Is there a reasonable remedy?
Is this node v6 support a non-goal for firestore at this point (we're using this with firestore and have had trouble with Google cloud functions in node 8 [we moved to node 8 and had to roll back] per this issue).

@mikelehen
Copy link
Contributor Author

mikelehen commented May 23, 2019 •
edited
Loading

@jeffreypriebe To be clear, are you using the firebase npm module (in which case this is the right github repo)? Or are you using firebase-admin or @google-cloud/firestore?

@mikelehen
Copy link
Contributor Author

mikelehen commented May 23, 2019 •
edited
Loading

@jeffreypriebe (I'm asking because usually you would use firebase-admin if you're using Cloud Functions...)

mikelehen pushed a commit that referenced this pull request May 23, 2019
Revert "Switch from 'grpc' to '@grpc/grpc-js' dependency. (#1804)"
494c564 
This reverts commit 4771184. Since this
changes our minimum node dependency from v6 to v8, it is a breaking change and
can't be done at this time.
Feiyang1 pushed a commit that referenced this pull request May 24, 2019
@Feiyang1
Revert "Switch from 'grpc' to '@grpc/grpc-js' dependency. (#1804)" (#…
ae32b2b 
…1813)

This reverts commit 4771184. Since this
changes our minimum node dependency from v6 to v8, it is a breaking change and
can't be done at this time.
@mikelehen
Copy link
Contributor Author

@jeffreypriebe We've ended up reverting this change (will be in the next release) since it is a breaking change to remove support for v6. We will hold off until our next breaking change release.

That said, node v6 has reached end-of-life (https://github.com/nodejs/Release#end-of-life-releases) and you should definitely strive to upgrade when you can.

nbransby added a commit to GitLiveApp/firebase-js-sdk that referenced this pull request Jun 14, 2019
@nbransby
Revert "Revert "Switch from 'grpc' to '@grpc/grpc-js' dependency. (fi…
5cc7363 
…rebase#1804)" (firebase#1813)"

This reverts commit ae32b2b.
@firebase firebase locked and limited conversation to collaborators Oct 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Feiyang1 Feiyang1 Feiyang1 approved these changes

@schmidt-sebastian schmidt-sebastian schmidt-sebastian approved these changes

@hiranya911 hiranya911 Awaiting requested review from hiranya911

@hsubox76 hsubox76 Awaiting requested review from hsubox76

Assignees

@Feiyang1 Feiyang1

Labels
api: firestore
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

FR: Replace grpc with grpc-js
4 participants
@mikelehen @jeffreypriebe @Feiyang1 @schmidt-sebastian