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

js: use commonjs_strict for well-known types #8955

Closed

Conversation

avm99963
Copy link

@avm99963 avm99963 commented Sep 6, 2021

When generating JS code for .proto files which included well-known types
with the commonjs_strict import style, the generated code would import
generated JS code for the well-known types with the commonjs import
style (located in the google-protobuf NPM package). This causes several
issues as discussed in protocolbuffers/protobuf-javascript#25.

This CL changes this so when using commonjs_strict, the imported
generated JS code for well-known types also uses commonjs_strict.

Fixes protocolbuffers/protobuf-javascript#25


Note: currently a folder named "commonjs_strict" is used to save the generated JS code for well-known types. I'm not fully convinced by this, so please let me know if you know how to improve where to store the generated code for the commonjs_strict mode. Thanks!

When generating JS code for .proto files which included well-known types
with the commonjs_strict import style, the generated code would import
generated JS code for the well-known types with the commonjs import
style (located in the google-protobuf NPM package). This causes several
issues as discussed in #7778.

This CL changes this so when using commonjs_strict, the imported
generated JS code for well-known types also uses commonjs_strict.

Fixes #7778
@avm99963
Copy link
Author

@acozzette would you mind taking a look at this and maybe review it? Thanks in advance! :)

@acozzette
Copy link
Member

We are closing all open JavaScript pull requests because the JavaScript implementation is moving to a new repository: https://github.com/protocolbuffers/protobuf-javascript Sorry for the inconvenience but please feel free to reopen the pull request in the new repo.

@acozzette acozzette closed this May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

js: protocols bundled with package are incompatible with commonjs_strict
5 participants