Protobuf: [JavaScript] Add insertion points to the compiler

[travikk]

What language does this apply to?
This feature request is for proto3 and JavaScript language generator.

Describe the problem you are trying to solve.

Problem I am trying to solve, is that when using gRPC (and generated proto files) in combination with create-react-app, the eslint complain about all the generated proto JS code with the following error reported all over the shop:
Line 29: 'proto' is not defined no-undef

Describe the solution you'd like

Since I already have my own plugin to help generate some other things along with JS, it'd be nice to have access to insertion points for the top and bottom of the file, so I could add the following myself:

/* eslint-disable */ and /* eslint-enable */ respectively

Describe alternatives you've considered

Ana lternative would be to have an option in the compiler that would add those for me, for instance:

      --js_out="import_style=commonjs,disable_eslint=true,binary:$OUTPUT_PATH"
      ${currentProto}

[BSBandme]

@haon4
There's one problem that internally js protobuf is implemented through plugin.

[ghost]

@dankurka
I feel like we should add /* eslint-disable / and / eslint-enable */ by default. Probably don't need to add insertion points for this.

[dankurka]

Before we try to suppress anything we should take a look at whether or not we can make eslint happy.

Why does lint not see the declaration of proto, could you add some output here or a way to reproduce?

[travikk]

@dankurka @haon4 here's a snippet that would produce eslint issues:

/**
 * @fileoverview
 * @enhanceable
 * @suppress {messageConventions} JS Compiler reports an error if a variable or
 *     field starts with 'MSG_' and isn't a translatable message.
 * @public
 */
// GENERATED CODE -- DO NOT EDIT!

var jspb = require('google-protobuf');
var goog = jspb;
var global = Function('return this')();

goog.exportSymbol('proto.com.PACKAGE_NAME_ERASED.AssetCategoryDto', null, global);

/**
 * Generated by JsPbCodeGenerator.
 * @param {Array=} opt_data Optional initial data array, typically from a
 * server response, or constructed directly in Javascript. The array is used
 * in place and becomes part of the constructed object. It is not cloned.
 * If no data is provided, the constructed object will be empty, but still
 * valid.
 * @extends {jspb.Message}
 * @constructor
 */
proto.com.PACKAGE_NAME_ERASED.AssetCategoryDto = function(opt_data) {
  jspb.Message.initialize(this, opt_data, 0, -1, null, null);
};
goog.inherits(proto.com.PACKAGE_NAME_ERASED.AssetCategoryDto, jspb.Message);
if (goog.DEBUG && !COMPILED) {
  proto.com.PACKAGE_NAME_ERASED.AssetCategoryDto.displayName = 'proto.com.PACKAGE_NAME_ERASED.AssetCategoryDto';
}

eslint will complain about the usage of "proto" global property, which to eslint is undefined.

[nagytech]

@dankurka

You probably just want to add /* global require, proto, COMPILED */ somewhere to tell eslint about the presence of globals.

Below is the snippet referenced by @travikk which no longer produces any errors.

/**
 * @fileoverview
 * @enhanceable
 * @suppress {messageConventions} JS Compiler reports an error if a variable or
 *     field starts with 'MSG_' and isn't a translatable message.
 * @public
 */

/*globals require, proto, COMPILED */

// GENERATED CODE -- DO NOT EDIT!

var jspb = require('google-protobuf');
var goog = jspb;
var global = Function('return this')();

goog.exportSymbol('proto.com.PACKAGE_NAME_ERASED.AssetCategoryDto', null, global);

/**
 * Generated by JsPbCodeGenerator.
 * @param {Array=} opt_data Optional initial data array, typically from a
 * server response, or constructed directly in Javascript. The array is used
 * in place and becomes part of the constructed object. It is not cloned.
 * If no data is provided, the constructed object will be empty, but still
 * valid.
 * @extends {jspb.Message}
 * @constructor
 */
proto.com.PACKAGE_NAME_ERASED.AssetCategoryDto = function(opt_data) {
  jspb.Message.initialize(this, opt_data, 0, -1, null, null);
};
goog.inherits(proto.com.PACKAGE_NAME_ERASED.AssetCategoryDto, jspb.Message);
if (goog.DEBUG && !COMPILED) {
  proto.com.PACKAGE_NAME_ERASED.AssetCategoryDto.displayName = 'proto.com.PACKAGE_NAME_ERASED.AssetCategoryDto';
}

[saturnism]

ran into this issue and unable to resolve w/o adding the /* eslint-disable */ :(

[travikk]

@dankurka

You probably just want to add /* global require, proto, COMPILED */ somewhere to tell eslint about the presence of globals.

Below is the snippet referenced by @travikk which no longer produces any errors.

/**
 * @fileoverview
 * @enhanceable
 * @suppress {messageConventions} JS Compiler reports an error if a variable or
 *     field starts with 'MSG_' and isn't a translatable message.
 * @public
 */

/*globals require, proto, COMPILED */

// GENERATED CODE -- DO NOT EDIT!

var jspb = require('google-protobuf');
var goog = jspb;
var global = Function('return this')();

goog.exportSymbol('proto.com.PACKAGE_NAME_ERASED.AssetCategoryDto', null, global);

/**
 * Generated by JsPbCodeGenerator.
 * @param {Array=} opt_data Optional initial data array, typically from a
 * server response, or constructed directly in Javascript. The array is used
 * in place and becomes part of the constructed object. It is not cloned.
 * If no data is provided, the constructed object will be empty, but still
 * valid.
 * @extends {jspb.Message}
 * @constructor
 */
proto.com.PACKAGE_NAME_ERASED.AssetCategoryDto = function(opt_data) {
  jspb.Message.initialize(this, opt_data, 0, -1, null, null);
};
goog.inherits(proto.com.PACKAGE_NAME_ERASED.AssetCategoryDto, jspb.Message);
if (goog.DEBUG && !COMPILED) {
  proto.com.PACKAGE_NAME_ERASED.AssetCategoryDto.displayName = 'proto.com.PACKAGE_NAME_ERASED.AssetCategoryDto';
}

Yeah exactly. But in order to be able to do that, we need some sort of insertion points to automate it, rather than having to modify the file time and time again after regenerating protos.

[nagytech]

@travikk Something like this then?

https://github.com/protocolbuffers/protobuf/pull/6439/commits/77fedcd1e38ad3c867a60d38a45e53ede27c5a74

[travikk]

perfect, thanks!

[travikk]

Actually, @nagytech that's not good enough. Once above issues got resolved, bunch of other errors started popping up, around things already defined or defined but not being used (no-redeclare and no-unsued-vars rules).

no-unsued-vars is reported on the following:

   proto.somepackage.MY_REQUEST.toObject = function(includeInstance, msg) {
      var f,
         obj = {
            procsList: jspb.Message.toObjectList(
               msg.getMyData(),
               someOtherPackageObscured.Proc.toObject,
               includeInstance,
            ),
            force: jspb.Message.getField(msg, 2),
            nonce: jspb.Message.getField(msg, 3),
         }

      if (includeInstance) {
         obj.$jspbMessageInstance = msg
      }
      return obj
   }

it's complaining about var f

no-redeclare is reported on the following:

```javascript
proto.somepackage.MY_REQUEST.deserializeBinaryFromReader = function(msg, reader) {
while (reader.nextField()) {
if (reader.isEndGroup()) {
break
}
var field = reader.getFieldNumber()
switch (field) {
case 1:
var value = new SOME_PACKAGE_OBSCURED.Proc()
reader.readMessage(value, SOME_PACKAGE_OBSCURED.Proc.deserializeBinaryFromReader)
msg.addMyData(value)
break
case 2:
var value = /* @type {boolean} */ reader.readBool()
msg.setOtherData(value)
break
case 3:
var value = /* @type {string} */ reader.readString()
msg.setSomeIntMaybe(value)
break
default:
reader.skipField()
break
}
}
return msg
}
````

it complains about var value = ...

[nagytech]

@travikk Maybe /* global require, proto, COMPILED */ worked for me because I had other settings in my tsconfig. Probably best to avoid the WOMM run around and just use /* eslint-disable */ instead.

[travikk]

@nagytech Yep, that would be my suggestion too. Do you think you can adjust https://github.com/protocolbuffers/protobuf/commit/77fedcd1e38ad3c867a60d38a45e53ede27c5a74 to allow disabling eslint instead of putting globals?

[nagytech]

@travikk

Done. https://github.com/protocolbuffers/protobuf/pull/6439/commits/ea34490cdb1b069f9a22f843dad97e3d21416c3e

[ribrdb]

It would be nice to have insertion points for other uses too. Examples might be a plugin generating services or adding proto3 json serialization.