grpc / grpc-web

gRPC for Web Clients
https://grpc.io
Apache License 2.0
8.57k stars 763 forks source link

Failed to compile. 'proto' is not defined (also 'COMPILED') #447

Open argoyb opened 5 years ago

argoyb commented 5 years ago

I'm using create-react-app with typescript and grpc-web, grpc & proto generated with command:

$(protoc) -I $(proto_dir) $(proto_dir)/ride.proto --js_out=import_style=commonjs:./rider/typescript/src/proto --grpc-web_out=import_style=commonjs+dts,mode=grpcwebtext:./rider/typescript/src/proto

App is not able to start, it throws tons of errors of form

Failed to compile.

./src/proto/ride_pb.js
  Line 37:     'proto' is not defined     no-undef
  Line 40:    'proto' is not defined     no-undef
  Line 41:    'COMPILED' is not defined  no-undef
  Line 42:     'proto' is not defined     no-undef
 ...
argoyb commented 5 years ago

I've tried to add src/proto to exclude section of package.json to exclude generated code from compilation but that didn't work. Probably because of following thing:

Any files that are referenced by files included via the "files" or "include" properties are also included. Similarly, if a file B.ts is referenced by another file A.ts, then B.ts cannot be excluded unless the referencing file A.ts is also specified in the "exclude" list.
argoyb commented 5 years ago

Was able to bypass this problem by adding /* eslint-disable */ to the top of generated ./src/proto/ride_pb.js. However it resulted in another error:

Failed to compile.

<path>/rider/typescript/src/proto/ride_grpc_web_pb.js
Type error: Cannot compile namespaces when the '--isolatedModules' flag is provided.  TS1208

    10 |
    11 |
  > 12 | const grpc = {};
       | ^
    13 | grpc.web = require('grpc-web');
    14 |
    15 | const proto = {};
argoyb commented 5 years ago

And I was able to bypass Cannot compile problem by adding // @ts-ignore to line 11 (above const grpc = {};)

stanley-cheung commented 5 years ago

Sorry for the late reply. TypeScript support is 'experimental' right now. We would definitely welcome contributions. Seems like you have resolved your issue?

argoyb commented 5 years ago

Well, I wouldn't call it "resolved" - I basically disabled typescript compiler check and eslint check. But true, it made project compile

caseylucas commented 5 years ago

@argoyb I have a similar setup as you (CRA, typescript, grpc-web). I generate the grpc-web related files into a separate github project, then include the generated files into my "main" project via yarn add https://github.com/ABC/XYZ.git. When included this way, you can still use typescript in your main project while also getting the grpc-web typescript definitions for you rendered client. I really did not want to have to modify the generated files by hand. With the method I mentioned there is no need to modify any generated files. My grpc-web options are similar to yours: --grpc-web_out=import_style=commonjs+dts,mode=grpcweb:.

hope this helps

ujjawaldx commented 5 years ago

And I was able to bypass Cannot compile problem by adding // @ts-ignore to line 11 (above const grpc = {};)

Is bypassing solution ?

Tim-S commented 5 years ago

@argoyb I have a similar setup as you (CRA, typescript, grpc-web). I generate the grpc-web related files into a separate github project, then include the generated files into my "main" project via yarn add https://github.com/ABC/XYZ.git. When included this way, you can still use typescript in your main project while also getting the grpc-web typescript definitions for you rendered client. I really did not want to have to modify the generated files by hand. With the method I mentioned there is no need to modify any generated files. My grpc-web options are similar to yours: --grpc-web_out=import_style=commonjs+dts,mode=grpcweb:.

hope this helps

Thanks, deploying the generated proto-sources in a separate npm-package works fine

(For development-purposes) i created a separate npm package "\<my-react-project>-proto-src" right next to "\<my-react-project>" and installed the dependency locally:

cd <my-react-project>
npm install ../<my-react-project>-proto-src

my grpc-web options are: --grpc-web_out=import_style=commonjs+dts,mode=grpcwebtext

robcecil commented 5 years ago

Btw, I get the same compile errors and I am not using Typescript.

If I simply edit the generated files by adding the / eslint-disable / at the top just like @argoyb . Be nice if there was some code gen flag to emit this.

raviprakashmishra commented 5 years ago

After adding / eslint-disable / to the start of the generated code, got rid of the compilation errors. But I see theses errors now - TypeError: Cannot read property '_handle' of undefined in /node_modules/grpc/node_modules/set-blocking/index.js:3

akesling commented 4 years ago

I was able to bypass the error by inserting a "fake" object at the top of the foo_pb.js file as such:

var proto = {
  foo: {
    ... nest the rest of the namespaces as appropriate ...
  }
}
var COMPILED;

NOTE: this only works for files generated for protos with no imports. If imports are present, the namespace resolution fails.

It seems clear to me that the Closure-style structure of a proto.foo.bar.v1.MyProto = ... isn't naively readable as either valid JS or TS because every part of proto.foo.bar.v1 is undefined. Same goes with the COMPILED symbol.

Is there some form of preamble that should be included as well? I.e. is there something being overlooked that provides the missing symbols?

This all works for me when compiled because it can resolve symbols before actually being dealt with "as javascript"'... but the codegen herein is advertised as "just working" without an intelligent compilation step. As generated, the proto module currently isn't valid javascript and will fail with a proto is not defined error as we're all seeing because the proto symbol hasn't been defined.

alpaka commented 4 years ago

Maybe a better workaround is to ignore _pb.js files for eslint, which doesn't require modifying the generated files every time (that gets old really fast).

Create a file called .env in your project root containing only EXTEND_ESLINT=true, then modify package.json to add ignorePatterns:

"eslintConfig": {
    "extends": "react-app",
    "ignorePatterns": ["**/*_pb.js"]
  }

Note that this is currently flagged experimental in the docs. Sources: https://create-react-app.dev/docs/setting-up-your-editor/ https://create-react-app.dev/docs/adding-custom-environment-variables/

akesling commented 4 years ago

Maybe a better workaround is to ignore _pb.js files for eslint, which doesn't require modifying the generated files every time (that gets old really fast).

@alpaka To clarify: for what I'm seeing, eslint is just warning of a deeper issue. This is not strictly an issue in eslint. Ignoring it at that layer prevents the warnings but does not prevent failure when the files are loaded in the browser.

The issue is that there is no included code which initializes the proper objects/namespaces. This happens for me when the _pb.js files are just loaded without any special symbol resolution (as seems to run for me when I compile).

nwparker commented 4 years ago

From reading earlier in this thread, I have found the full workaround to be the following:

  1. Use TypeScript 2.7+ (it's in beta now, but it's needed for this to work)
  2. Add /*eslint-disabled */ and //@ts-nocheck to the top of each of the generated .js files

For the second step, it's easier to do this in a script. You can use something similar to what was suggested here. For example:

for f in "${out}"/*.js; do
    printf '/* eslint-disable */\n//@ts-nocheck\n' | cat - "${f}" > temp && mv temp "${f}"
done

Hopefully a proper solution arises in the future

stanley-cheung commented 4 years ago

Hope this has been fixed by #752

mcardinalupgrade commented 4 years ago

Given an initial test.proto file, it looks like https://github.com/grpc/grpc-web/pull/752 has fixed the:

  Line 28:1:    'proto' is not defined     no-undef
  Line 29:50:   'proto' is not defined     no-undef
  Line 31:15:   'proto' is not defined     no-undef
  Line 32:20:   'COMPILED' is not defined  no-undef
  Line 37:3:    'proto' is not defined     no-undef

issues that were seen in the test_grpc_web_pb.js file but the header is not being added to the test_pb.js and the errors are unfortunately still present.

ADustyOldMuffin commented 4 years ago

I can also confirm, the generated ts file for a service has the /* eslint-disable */ and // @ts-nocheck up top but the pb.js file does not and still throws errors. Would it not be better to just initialize dummy objects? I used the solution suggested by @alpaka and it works fine. If anything this is the way to fix it.

stanley-cheung commented 4 years ago

Unfortunately the _pb.js files are generated by the --js_out part of the protoc command, which is not owned by us here in grpc-web. We can only control the _grpc_web_pb.js output generated by the --grpc-web_out part of the protoc command.

c10b10 commented 4 years ago

@stanley-cheung who is the generation of that file controlled by?

jscarl commented 3 years ago

I've solved this issue by adding EXTEND_ESLINT=true in the .env file