-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Add empty mock for http2 #5686
Add empty mock for http2 #5686
Conversation
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
@kjin what happened to this PR? I stumbled across the same issue with firebase-admin. |
At the time I wasn't sure if this would just be papering over a bigger problem. Though, it might still be good to get this in first, so that the real problem (that google-gax depends transitively on http2, even if it doesn't get compiled into the app itself) can get fixed downstream. |
I think you will need to email cla@fb.com then. |
Edit: jk, it's there |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Looks like that worked. |
This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs. |
@iansu anything I need to do to advance the state of this PR? Thanks! |
Hi, want this feature to be merged very much! |
ping @iansu |
@kjin is there temporary fix to solve this issue? |
@zeeshancb This is probably as "temporary" as it gets, it papers over a problem that really lies in the usage of certain modules -- as HTTP2 is not implemented in the browser, if you're using a module that depends on it, either (a) you should not be using it or (b) the module authors themselves should be taking steps to making their module web-compatible, whether that means conditionally requiring |
Looks like this broke CI. |
I think the test failures are unrelated. Seeing the same error on some other PRs. |
This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs. |
This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs. |
Thanks! |
* masterd: (24 commits) Add TypeScript linting support (facebook#6513) Support React Hooks (facebook#5602) (facebook#5997) Support browserslist in @babel/preset-env (facebook#6608) Add empty mock for http2 (facebook#5686) Add note about npx caching (facebook#6374) change named import into default import (facebook#6625) Stage files for commit after ejecting (facebook#5960) Upgrade dependencies (facebook#6614) Make compiler variable const instead of let (facebook#6621) Type check JSON files (facebook#6615) Change class components to functional components in templates (facebook#6451) Convert JSON.stringify \n to os.EOL when writing tsconfig.json (facebook#6610) Update html-webpack-plugin (facebook#6361) Enable click to go to error in console for TypeScript (facebook#6502) Update webpack-dev-server to 3.2.1 (facebook#6483) [docs] revert removal of newlines from html (facebook#6386) Publish Prepare 2.1.8 release Reapply "Speed up TypeScript v2 (facebook#6406)" (facebook#6586) Publish ... # Conflicts: # packages/babel-preset-react-app/create.js # packages/react-scripts/scripts/build.js
Following suit with #2600, this PR adds
http2
as an empty module in webpack configs.I don't seehttp
to the list, which suggests that this empty mock should probably be replaced with a browser-side implementation sometime in the future (I can add a TODO if needed).See: grpc/grpc-node#610 (Even though it turns out the module in question seems to not be meant for the client side, I believe the lack of this mock masked the real issue)