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 CommonJS support as a plugin option #587
Comments
From this Slack thread, without BSR support for CommonJS (and, by extension, NestJS), the "best" workaround is to bypass BSR completely and instead create a monorepo with CI builds that generate CommonJS NPM packages. Is this true? If so, how likely is Buf to support CommonJS and would we be talking months or years? If this is farther away than a couple of months, we'll have to move ahead with doing all of this ourselves and removing BSR from our workflow. Sadly, NestJS has no plans to update to ESM. |
Hey Rocky, I am not sure that bypassing the BSR completely is the best workaround. NestJS does not support ESM-only npm packages, but it does support TypeScript files with ESM imports. For example, you might have an import like the following in your app: // app.module.ts
import { AppController } from './app.controller'; This works in Nest, because all source files are transpiled to CJS. If you generate code locally with protoc-gen-es, you can set the option It isn't as neat as remote packages, but you can generate local code from remote protobuf files on the BSR. You need one configuration file: # buf.gen.yaml
version: v1
plugins:
- plugin: buf.build/bufbuild/es
out: src/gen
opt: target=ts
- plugin: buf.build/connectrpc/es
out: src/gen
opt: target=ts Then you can run $ buf generate buf.build/connectrpc/eliza
$ tree src/gen
src/gen
└── connectrpc
└── eliza
└── v1
├── eliza_connect.ts
└── eliza_pb.ts
3 directories, 2 files Your use case might vary, but right now, this would be my recommended approach with the BSR in case the framework does not support ESM. |
This should not be necessary. Import statements are generated by protobuf-es/packages/protoplugin/src/ecmascript/generated-file.ts Lines 206 to 209 in fcf1a1e
This could easily be extended to support CommonJS The more challenging part are export statements. They are currently handwritten by plugins. In order to support CommonJS with a plugin option, we should provide an API to generate export statements, which automatically produces a ECMAScript export statement or a CommentJS export assignment, based on the plugin option. |
@timostamm Thanks for the reply! Our use case: We define EventBridge message schemas with Protobuf. Each message is sent from a single The ideal flow is as follows:
Since we can't use the BSR-generated assets, however, the flow is:
This works fine if a Assuming we're generating these files into the same location, say This solution also means every The above seems like a high price to pay just to keep using BSR as it provides minimal benefit for this use case. Instead, it seems less error prone and produces less cognitive load for microservice developers to move Protobuf schemas to a monorepo that generates the NPM packages. This gets us pretty close to the "ideal flow" above with the following downsides:
|
Thanks for sharing the details on the use case, @therockstorm. You can re-use We have a couple of other features in the pipeline, but this will be implemented. It will most likely land before the end of the year. |
That's excellent news, @timostamm! I was using https://github.com/timostamm/protobuf-ts prior to https://github.com/bufbuild/protobuf-es and they, along with the rest of the Buf tooling, are excellent to use. Thanks to the team for their efforts and for your helpful, timely responses on this issue. |
Wow, I really needed this option! Thanks! |
Hi @krapie we just published v1.6.0 to npm: https://www.npmjs.com/package/@bufbuild/protobuf |
@smaye81 Thank you for the release! But I'm still wondering why my |
@krapie, protoc-gen-connect-es wasn't updated to support the option yet. But if you update to the just released v1.2.0, it should generate CommonJS exports too: version: v1
plugins:
# You'll need @bufbuild/protoc-gen-es v1.6.0 or later,
# and @bufbuild/protoc-gen-connect-es v1.2.0 or later
- plugin: es
out: src/gen
opt: js_import_style=legacy_commonjs
- plugin: connect-es
out: src/gen
opt: js_import_style=legacy_commonjs We don't have integration tests for this plugin option, but it should do the job. Let us know if you encounter any problems 🙂 |
@timostamm Ohh, I thought this release updates all plugins to provide new option (I was able to use It is now working perfectly. Thank you for your fast feedback and release! |
Just to provide context: All plugins based on the plugin framework But the plugin framework can only modify export statements for CommonJS when it knows what the export statements are. For example, protoc-gen-connect-es was using this call to generate a service: f.print("export const ", localName(service), " = {");
f.print(f.exportDecl("const", localName(service)), " = {"); Similar to the already existing |
The current code generators (like
bufbuild/protobuf-es
andconnectrpc/es
) support ESM only. What do you think about adding an opt-in plugin option to modify the default behaviour?As this project becomes more popular, it'd be nice for the generated code to "play nice" with the rest of the ecosystem. For example, some projects like NestJS do not work well with ESM-only. And the alternatives aren't great, such as importing the module asynchronously.
Given that the Buf Schema Registry (BSR) hosts plugins with a pre-configured set of options, it means we'd need to publish a second plugin named based on the import. This isn't great, so something to think about.
https://buf.build/bufbuild/es vs https://buf.build/bufbuild/es-commonjs (does not exist)
Lastly, since we'll want this to be a "registry-compatible" plugin, we'll need a commonjs implementation for rewriting import paths.
The text was updated successfully, but these errors were encountered: