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

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

Open
argoyb opened this issue Jan 30, 2019 · 20 comments
Open

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

argoyb opened this issue Jan 30, 2019 · 20 comments

Comments

@argoyb
Copy link

argoyb commented Jan 30, 2019

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
Copy link
Author

argoyb commented Jan 30, 2019

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
Copy link
Author

argoyb commented Jan 30, 2019

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
Copy link
Author

argoyb commented Jan 30, 2019

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

@stanley-cheung
Copy link
Collaborator

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

@argoyb
Copy link
Author

argoyb commented Feb 4, 2019

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

@caseylucas
Copy link
Collaborator

@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
Copy link

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

Is bypassing solution ?

@Tim-S
Copy link

Tim-S commented May 12, 2019

@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
Copy link

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
Copy link

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
Copy link

akesling commented Dec 10, 2019

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
Copy link

alpaka commented Dec 23, 2019

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
Copy link

akesling commented Dec 23, 2019

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
Copy link

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
Copy link
Collaborator

Hope this has been fixed by #752

@mcardinalupgrade
Copy link

mcardinalupgrade commented Jun 23, 2020

Given an initial test.proto file, it looks like #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
Copy link

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 stanley-cheung reopened this Jul 8, 2020
@stanley-cheung
Copy link
Collaborator

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
Copy link

c10b10 commented Jul 14, 2020

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

@jscarl
Copy link

jscarl commented Sep 25, 2020

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

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

No branches or pull requests