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.

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

Missing dependency declaration for @bufbuild/protobuf #432

Closed
kubijo opened this issue Jan 25, 2023 · 16 comments · Fixed by #496
Closed

Missing dependency declaration for @bufbuild/protobuf #432

kubijo opened this issue Jan 25, 2023 · 16 comments · Fixed by #496
Labels
bug Something isn't working

Comments

@kubijo
Copy link

kubijo commented Jan 25, 2023

Describe the bug

Running this plugin in context of Yarn V3 with enabled PNP mechanism produces the following error:

 Generate files 
 ============== 
Error: Command failed: /xxx/.yarn/unplugged/node-protoc-npm-1.0.3-639f7ab0b0/node_modules/node-protoc/dist/protoc/bin/protoc --es_out=/xxx/packages/toolbox/gen --es_opt=target=ts,import_extension=none --connect-web_out=/xxx/packages/toolbox/gen --connect-web_opt=target=ts,import_extension=none --proto_path=/xxx/packages/toolbox/ext/main/proto /xxx/packages/toolbox/ext/main/proto/google/rpc/error_details.proto /xxx/packages/toolbox/ext/main/proto/google/rpc/status.proto /xxx/packages/toolbox/ext/main/proto/core/news.proto /xxx/packages/toolbox/ext/main/proto/core/net.proto /xxx/packages/toolbox/ext/main/proto/core/monitoring.proto /xxx/packages/toolbox/ext/main/proto/core/error.proto /xxx/packages/toolbox/ext/main/proto/core/money.proto /xxx/packages/toolbox/ext/main/proto/core/fido.proto /xxx/packages/toolbox/ext/main/proto/core/rewards.proto /xxx/packages/toolbox/ext/main/proto/core/payouts.proto /xxx/packages/toolbox/ext/main/proto/core/mailing.proto /xxx/packages/toolbox/ext/main/proto/core/activities.proto /xxx/packages/toolbox/ext/main/proto/core/client_errors.proto /xxx/packages/toolbox/ext/main/proto/core/fingerprint.proto /xxx/packages/toolbox/ext/main/proto/core/communication.proto /xxx/packages/toolbox/ext/main/proto/core/mining.proto /xxx/packages/toolbox/ext/main/proto/core/captcha.proto /xxx/packages/toolbox/ext/main/proto/core/otp.proto /xxx/packages/toolbox/ext/main/proto/core/client.proto /xxx/packages/toolbox/ext/main/proto/toolbox/service.proto
google/rpc/error_details.proto:19:1: warning: Import google/protobuf/duration.proto is unused.
/xxx/.pnp.cjs:40517
      Error.captureStackTrace(firstError);
            ^

Error: @bufbuild/protoc-gen-connect-web tried to access @bufbuild/protobuf, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

Required package: @bufbuild/protobuf
Required by: @bufbuild/protoc-gen-connect-web@virtual:a9df22bd2675136058ccb33218a730656dcf048baefeca725877b87d0548fc929e41a7472e015620fd882f1dd55b43b92022f5949aa7e54a8098679bdfca0251#npm:0.6.0 (via /xxx/.yarn/unplugged/@bufbuild-protoc-gen-connect-web-virtual-ff7bfa0b23/node_modules/@bufbuild/protoc-gen-connect-web/dist/cjs/src/)

Require stack:
- /xxx/.yarn/unplugged/@bufbuild-protoc-gen-connect-web-virtual-ff7bfa0b23/node_modules/@bufbuild/protoc-gen-connect-web/dist/cjs/src/typescript.js
- /xxx/.yarn/unplugged/@bufbuild-protoc-gen-connect-web-virtual-ff7bfa0b23/node_modules/@bufbuild/protoc-gen-connect-web/dist/cjs/src/protoc-gen-connect-web-plugin.js
- /xxx/.yarn/unplugged/@bufbuild-protoc-gen-connect-web-virtual-ff7bfa0b23/node_modules/@bufbuild/protoc-gen-connect-web/bin/protoc-gen-connect-web
    at require$$0.Module._resolveFilename (/xxx/.pnp.cjs:40517:13)
    at require$$0.Module._load (/xxx/.pnp.cjs:40368:42)
    at Module.require (node:internal/modules/cjs/loader:1105:19)
    at require (node:internal/modules/cjs/helpers:103:18)
    at Object.<anonymous> (/xxx/.yarn/unplugged/@bufbuild-protoc-gen-connect-web-virtual-ff7bfa0b23/node_modules/@bufbuild/protoc-gen-connect-web/dist/cjs/src/typescript.js:17:20)
    at Module._compile (node:internal/modules/cjs/loader:1218:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1272:10)
    at require$$0.Module._extensions..js (/xxx/.pnp.cjs:40561:33)
    at Module.load (node:internal/modules/cjs/loader:1081:32)
    at require$$0.Module._load (/xxx/.pnp.cjs:40399:14)

Node.js v19.3.0
--connect-web_out: protoc-gen-connect-web: Plugin failed with status code 1.

    at ChildProcess.exithandler (node:child_process:419:12)
    at ChildProcess.emit (node:events:513:28)
    at maybeClose (node:internal/child_process:1098:16)
    at ChildProcess._handle.onexit (node:internal/child_process:304:5) {
  code: 1,
  killed: false,
  signal: null,
  cmd: '/xxx/.yarn/unplugged/node-protoc-npm-1.0.3-639f7ab0b0/node_modules/node-protoc/dist/protoc/bin/protoc --es_out=/xxx/packages/toolbox/gen --es_opt=target=ts,import_extension=none --connect-web_out=/xxx/packages/toolbox/gen --connect-web_opt=target=ts,import_extension=none --proto_path=/xxx/packages/toolbox/ext/main/proto /xxx/packages/toolbox/ext/main/proto/google/rpc/error_details.proto /xxx/packages/toolbox/ext/main/proto/google/rpc/status.proto /xxx/packages/toolbox/ext/main/proto/core/news.proto /xxx/packages/toolbox/ext/main/proto/core/net.proto /xxx/packages/toolbox/ext/main/proto/core/monitoring.proto /xxx/packages/toolbox/ext/main/proto/core/error.proto /xxx/packages/toolbox/ext/main/proto/core/money.proto /xxx/packages/toolbox/ext/main/proto/core/fido.proto /xxx/packages/toolbox/ext/main/proto/core/rewards.proto /xxx/packages/toolbox/ext/main/proto/core/payouts.proto /xxx/packages/toolbox/ext/main/proto/core/mailing.proto /xxx/packages/toolbox/ext/main/proto/core/activities.proto /xxx/packages/toolbox/ext/main/proto/core/client_errors.proto /xxx/packages/toolbox/ext/main/proto/core/fingerprint.proto /xxx/packages/toolbox/ext/main/proto/core/communication.proto /xxx/packages/toolbox/ext/main/proto/core/mining.proto /xxx/packages/toolbox/ext/main/proto/core/captcha.proto /xxx/packages/toolbox/ext/main/proto/core/otp.proto /xxx/packages/toolbox/ext/main/proto/core/client.proto /xxx/packages/toolbox/ext/main/proto/toolbox/service.proto'
}

but adding the following section to .yarnrc.yml resolves the issue

packageExtensions:
  "@bufbuild/protoc-gen-connect-web@*":
    peerDependencies:
      "@bufbuild/protobuf": "*"

Environment (please complete the following information):

  • @bufbuild/protoc-gen-connect-web: 0.6.0

Additional context

$ yarn --version
4.0.0-rc.34.git.20221220.hash-3246d10

$ node --version
v19.3.0
@kubijo kubijo added the bug Something isn't working label Jan 25, 2023
@thesiti92
Copy link

similarly, i tried to get connect web working in my project and everything installed fine without @bufbuild/protobuf but type inference was broken

@kubijo
Copy link
Author

kubijo commented Feb 25, 2023

Just a friendly reminder… This is still a problem with @bufbuild/protoc-gen-connect-es

@unicomp23
Copy link

https://github.com/bufbuild/connect-es/blob/79c385fc417094c2681cc02b00ac269b1499ee54/packages/protoc-gen-connect-es/README.md#import_extensionjs

I added this to the "opt:" list in buf.yaml, and things started working. Maybe it'll help here too?

@smaye81
Copy link
Member

smaye81 commented Feb 28, 2023

@kubijo I haven't been able to recreate this issue with Yarn v3 and below. I did notice you're using an RC of Yarn 4, though. Can you provide some additional information?

  • Are you using buf or protoc to generate the files?
  • How are you invoking the generate step? i.e. via a package.json script?
  • How did you install the RC of Yarn 4 that you have

@kubijo
Copy link
Author

kubijo commented Feb 28, 2023

Are you using buf or protoc to generate the files?

The latest protoc downloaded from their GitHub releases page.

yarn node ./bin/protoc.js --version
libprotoc 22.0

How are you invoking the generate step? i.e. via a package.json script?

The whole script is rather lengthy, but it can be summarized thusly:

# https://github.com/bufbuild/protobuf-es/tree/main/packages/protoc-gen-es#with-protoc
PATH="$(dirname "$(yarn bin protoc-gen-es)"):$PATH"
# https://github.com/bufbuild/connect-web/tree/main/packages/protoc-gen-connect-web#with-protoc
PATH="$(dirname "$(yarn bin protoc-gen-connect-es)"):$PATH"
export PATH

yarn node "$DIR_ROOT/bin/protoc.js" \
  --es_out="${DIR_PROTO_OUT}" \
  --es_opt="target=ts,import_extension=.js" \
  --connect-es_out="${DIR_PROTO_OUT}" \
  --connect-es_opt="target=ts,import_extension=.js" \
  --proto_path="${DIR_PROTO_INP}" \
  "${files[@]}"

How did you install the RC of Yarn 4 that you have

I believe that the command used to be yarn set version from sources or something like that, but looking into the documentation, it is now probably yarn set version canary.

I have tried just now with these versions and the results were as mentioned already.
The reproduction consisted of:

  • setting version (as seen below)
  • yarn install (omitted for brevity)
  • running the generation script
frontend ❯ yarn set version latest
➤ YN0000: Downloading https://repo.yarnpkg.com/3.4.1/packages/yarnpkg-cli/bin/yarn.js
➤ YN0000: Saving the new release in .yarn/releases/yarn-3.4.1.cjs
➤ YN0000: Done in 0s 138ms
frontend ❯ yarn set version canary
➤ YN0000: Retrieving https://repo.yarnpkg.com/4.0.0-rc.39/packages/yarnpkg-cli/bin/yarn.js
➤ YN0000: Saving the new release in .yarn/releases/yarn-4.0.0-rc.39.cjs
➤ YN0000: Done in 0s 568ms

Both resulted in this (among other output):

Error: @bufbuild/protoc-gen-connect-es tried to access @bufbuild/protobuf, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

The simple fact is, though, that you do import @bufbuild/protobuf here, but do not declare the dependency.

This will work with NPM and other lenient package managers because @bufbuild/protobuf will be installed practically every time because it's a dependency in some other package you install and/or you declare it yourself in your package, as the error mentions this is not a sound way of distribution because the package is using something it doesn't disclose as a dependency.

Why you haven't been able to reproduce the issue is a mystery to me… However, I am certain that just adding the dependency will resolve this since that is what yarn does when you use the packageExtensions option to augment the package.

@smaye81
Copy link
Member

smaye81 commented Feb 28, 2023

Thanks for the info. And yeah, I understand what you're saying. The reason I'm asking is because I'm curious why some of our tests and examples (that use Yarn PnP) didn't catch this problem. The fix makes sense, I just don't know why I can't recreate it.

@kubijo
Copy link
Author

kubijo commented Feb 28, 2023

Yeah man, it confuses me too... Possibly someone from yarn team would be of help... I find them being extremely helpful and willing to help.

@fubhy
Copy link
Contributor

fubhy commented Feb 28, 2023

@smaye81 I can reproduce it too. JFYI :)

@smaye81
Copy link
Member

smaye81 commented Mar 1, 2023

I was able to reproduce it too. There was a few things wrong with our Yarn project. @fubhy stamped your PR. Thank you for the fix.

@smaye81 smaye81 closed this as completed Mar 1, 2023
@kubijo
Copy link
Author

kubijo commented Mar 1, 2023

@smaye81 Do you have an idea when I can expect a release? Not that it's particularly holding me back… I'd just like to clean my configs & get a feeling for your releasing policies… Oh, and thanks for the kind responses & prompt fix 👍

@smaye81
Copy link
Member

smaye81 commented Mar 1, 2023

Hopefully releasing this today! Stay tuned.

EDIT: Apologies for the delay @kubijo. Working out a few more things internally before the release. Might be looking at sometime tomorrow.

@smaye81
Copy link
Member

smaye81 commented Mar 6, 2023

@kubijo We finally published our latest release v0.8.2. This dependency issue should be fixed. Appreciate your patience.

@kubijo
Copy link
Author

kubijo commented Mar 7, 2023

@smaye81 Sadly, there seems to be a regression with @bufbuild/connect-web at 0.8.2 where typescript now doesn't see the module at all…

Example import being:

import { createGrpcWebTransport } from '@bufbuild/connect-web';

It did work with 0.8.1 obviously.
A completely missing dist directory might be a viable candidate for the core of the problem

@kubijo
Copy link
Author

kubijo commented Mar 7, 2023

Oh, I see you've got that already in #514... never mind then

@timostamm
Copy link
Member

Apologies. Just published v0.8.2-1 of the affected packages. npm i @bufbuild/connect-web@latest should update you to the fixed release.

A completely missing dist directory might be a viable candidate for the core of the problem

Let's call it aggressive bundle size optimization 😆

@kubijo
Copy link
Author

kubijo commented Mar 7, 2023

Your sense of humour is not lost on me nor misplaced for these kinds of things 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants