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

Can not start app using grpc web due to incorrect eslint errors #7295

Open
Globegitter opened this issue Jun 28, 2019 · 5 comments
Open

Can not start app using grpc web due to incorrect eslint errors #7295

Globegitter opened this issue Jun 28, 2019 · 5 comments

Comments

@Globegitter
Copy link

Desribe the bug

I am creating a react app with grpc-web: https://github.com/grpc/grpc-web

Following the steps described here https://github.com/grpc/grpc-web#how-it-works it generates code for you from the .proto files. If one has a helloworld.proto it would generate a helloworld_pb.js file. The file it generates confuses eslint and throws a bunch of no-undef errors which fails the build. If I manually add /* eslint disable no-undef */ to the generated file the App works as expected with grpc-web

Here is the generated file that is causing issues for me: https://gist.github.com/Globegitter/c53de0ba359cea1b2cc7c7cdfb0ac491

I do not want to add the eslint comment there because the file can be regenerated at any time and it also tells you to not manually edit the file.

Did you try recovering your dependencies?

Yep

Which terms did you search for in User Guide?

lint
eslint

Environment

Environment Info:

System:
OS: Linux 4.15 elementary OS 5.0 Juno (Ubuntu 18.04)
CPU: (8) x64 Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz
Binaries:
Node: 10.16.0 - /usr/bin/node
Yarn: 1.16.0 - /usr/bin/yarn
npm: 6.9.0 - /usr/bin/npm
Browsers:
Chrome: 75.0.3770.100
Firefox: Not Found
npmPackages:
react: ^16.8.6 => 16.8.6
react-dom: ^16.8.6 => 16.8.6
react-scripts: 3.0.1 => 3.0.1
npmGlobalPackages:
create-react-app: Not Found

Steps to reproduce

  1. Write any react app that imports the file linked in the gist e.g. import {HelloReply, HelloRequest} from './helloworld_pb';
  2. You can make use of this, instantiate the class etc.
  3. See the compilation issue thrown by eslint
  4. Add the eslint disable command and see that all works fine as expected

Note: I am using react with typescript here btw, but don't think that makes any difference here.

Expected behavior

Give me some way to ignore specific files/directories from being linted, or give me an option turn errors into warnings so I can choose to ignore false positives.

Actual behavior

image

Reproducible demo

https://github.com/Globegitter/react-grpc-web

@Globegitter Globegitter changed the title Can not start app using grpc web due to eslint Can not start app using grpc web due to incorrect eslint errors Jun 28, 2019
@heyimalex
Copy link
Contributor

Give me some way to ignore specific files/directories from being linted, or give me an option turn errors into warnings so I can choose to ignore false positives.

The maintainers are averse to adding more configuration to CRA, especially when there are already multiple ways to handle it. I know it sucks, but I think it'd be smart to try and find another way. no-undef is an incredibly common eslint rule; maybe try lobbying at grpc-web for a flag that adds the ignore line for you.

@Globegitter
Copy link
Author

To be honest I am happy with any fix that would be accepted, those are just the things I could think of. But you think there is nothing that could/would be changed here?

@karbica
Copy link

karbica commented Jul 12, 2019

@Globegitter I am suffering from the same issue as well. This is entirely on the CRA team and their position to keep the .eslintignore file from being exposed/supported to the developer.

I can't seem to find any other work around other than what was discussed here. Both parties are going to be pretty stubborn about this.

So, adding that line manually can be tedious if you need an automated solution. What I decided to do was immediately transform the files generated by protoc. You can then have a script in your package.json that executes some Bash code to prepend the required line /* eslint-disable */.

#!/usr/bin/env bash

# pb_disable_eslint.sh
# you can use it like so:
# ./pb_disable-eslint.sh ./my/path/to/proto/dir

for F in $(ls -A1 $1)
do
echo "Prepending file: $F"
echo '/* eslint-disable */' | cat - $F > temp && mv temp $F
done

If you are uncomfortable with the Bash file, you can use glob and the native fs module to modify the output directory of protoc.

This all would have been easier if we could just control our own .eslintignore file just like all the other ignore files.

@Globegitter
Copy link
Author

Yeah, I came up with a similar solution in the end - we are just using a gradle task in combination with https://github.com/google/protobuf-gradle-plugin for it. Would be nice to have something more permanent but at least it is workable.

@thepont
Copy link

thepont commented Dec 8, 2019

Just had this issue was well and opted for changing the scripts in the package.json as such. ( this tells the react script to not ignore your changes to the eslint config. ( perplexing that this isn't the default )

  "scripts": {
    "start": "EXTEND_ESLINT=true react-scripts start",
    "build": "EXTEND_ESLINT=true react-scripts build",
    "test": "react-scripts test",
    "eject": "react-scripts eject"
  }

and then modifying the eslintrc to ignore protobuf files.

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

This was made possible due to a fix in #6f5221c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants