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

[ts_proto_library] - Does not support Nodejs #562

Closed
Toxicable opened this issue Dec 24, 2018 · 3 comments · Fixed by #1593
Closed

[ts_proto_library] - Does not support Nodejs #562

Toxicable opened this issue Dec 24, 2018 · 3 comments · Fixed by #1593

Comments

@Toxicable
Copy link

🐞 bug report

Affected Rule

ts_proto_library

Is this a regression?

Nope

Description

The module format of the code generated is not usable in a Nodejs environment due to it being AMD module format, where as Nodejs expects CommonJS.

From looking at the docs: https://github.com/dcodeIO/ProtoBuf.js/#pbts-for-typescript
They don't appear to support different modules so either a preprocessing step to change the module format is required to changing the protobufjs lib itself to support other module code generation.

🔬 Minimal Reproduction

angular/universal#1003

🔥 Exception or Error

Error: ReferenceError: define is not defined

This is the generated code:

define('nguniversal/modules/grpc-engine/src/engine_proto_ts', ["protobufjs/minimal"], function($protobuf) {
    "use strict"'
    var $Reader = $protobuf.Reader, $Writer = $protobuf.Writer, $util = $protobuf.util;

🌍 Your Environment

Operating System:

  Mac OS X

Output of bazel version:

Build label: 0.20.0-homebrew
Build target: bazel-out/darwin-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Wed Dec 19 01:10:31 2018 (1545181831)
Build timestamp: 1545181831
Build timestamp as int: 1545181831

Rules version (SHA):

  

  

Anything else relevant?

@alexeagle alexeagle transferred this issue from bazelbuild/rules_typescript Feb 22, 2019
@qzmfranklin
Copy link
Contributor

qzmfranklin commented Aug 11, 2019

There is a simple fix to this issue.

You just need to tweak the following line in ts_proto_library.bzl:

def _run_pbjs(actions, executable, output_name, proto_files, suffix = '.js',
              wrap = 'amd', amd_name = ''):

Change amd to default and it will work.

I am thinking of upstreaming my changes here. Just have not got enough priorities yet. If it works for you and there is a strong interest, please feel free to bring up a PR.

@qzmfranklin
Copy link
Contributor

FYI, our team has been using this trick for a few months and it served us well.

@KingDarBoja
Copy link

Just to update, the ts_proto_library has been moved on PR #1159.

New path to ts_proto_library.bzl.

Cheers!

alexeagle pushed a commit that referenced this issue Feb 5, 2020
* change ts_proto_library to default es5 wrapping

This change wraps es5 sources generated with `ts_proto_library` to be wrapped in "default" mode. The "default" mode behaves like UMD, supporting both AMD and CommonJS. This change is necessary to support Node, as Node does not use AMD. See #562
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants