js: protocols bundled with package are incompatible with commonjs_strict

[ChALkeR]

This issue is about google-protobuf@3.12.4 js package, as published on npm.

Currently, it contains the following files:

./README.md
./google-protobuf.js
./package.json
./google/protobuf/any_pb.js
./google/protobuf/empty_pb.js
./google/protobuf/descriptor_pb.js
./google/protobuf/field_mask_pb.js
./google/protobuf/type_pb.js
./google/protobuf/duration_pb.js
./google/protobuf/source_context_pb.js
./google/protobuf/struct_pb.js
./google/protobuf/api_pb.js
./google/protobuf/compiler/plugin_pb.js
./google/protobuf/timestamp_pb.js
./google/protobuf/wrappers_pb.js

./google/ protocols are built in commonjs mode as opposed to commonjs_strict, which makes them both pollute the global scope (they create a global proto variable) and not compatible with CSP (they use Function constructor):

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

goog.exportSymbol('proto.google.protobuf.Any', 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.google.protobuf.Any = function(opt_data) {

I would have expected commonjs_strict protocols to be present there, either under the same folder, or perhaps in a separate google/protobuf.strict or strict/google/protobuf directory.

That would make them usable in commonjs_strict mode.

[perezd]

We'd be happy for you to submit a PR for this!

[dlj-NaN]

Both protocolbuffers/protobuf#5464 (Dec 13, 2018, 3 thumbs up) and protocolbuffers/protobuf#6770 (Oct 15, 2019, 8 thumbs up) are similar. I have closed them in favor of this issue, since I think @ChALkeR has provided a great staring point here.

[JulesPatry]

Any update on this?

[arzmir]

Seing as @lukesandberg was assigned to this issue. Is it safe to assume there is some traction here @perezd?

[MarnixBouhuis]

I've created a PR that fixes this issue partially, protocolbuffers/protobuf#8864 fixes the CSP issue, but does not generate the protobufs in ./google/ with commonjs_strict.

[avm99963]

@MarnixBouhuis I've submitted PR protocolbuffers/protobuf#8955 which complements yours, to generate the well-known types code with the commonjs_strict import style :)

[eKazim]

It would be nice to merge to master PR protocolbuffers/protobuf#8955 together with the fix of protocolbuffers/protobuf-javascript#40 (PR protocolbuffers/protobuf#8696), because otherwise it is possible that it will still be impossible to use this modules generated with commonjs_strict. But unfortunately, PRs for javascript are ignoring for a long time...

[avm99963]

But unfortunately, PRs for javascript are ignoring for a long time...

I just noticed your PR is stuck for review since June :(( It'd be awesome if some maintainer could review all the PRs mentioned in this issue! (#8696, protocolbuffers/protobuf#8864, protocolbuffers/protobuf#8955)

otherwise it is possible that it will still be impossible to use this modules generated with commonjs_strict

Could you expand on that? Right now I don't notice how this could happen, so I might be missing something.

[eKazim]

Could you expand on that? Right now I don't notice how this could happen, so I might be missing something.

According to the example in the comment , there will be extra nesting of package in the export of the js module. So, when you will try to use the current generated modules in another js module by importing them (require()), it will return undefined instead of the desired structure. But perhaps, this problem is relevant for a deeper nesting of the package, and maybe with package google.protobuf there is no problem. In theory, it is easy to check this by collecting these modules manually and trying to use them in another module.

[dibenede]

I'm currently looking at some packaging issues and can take a look at this.

[unicornonea]

Any update on this issue?

[lukesandberg]

i think it would be reasonable to start releasing the well known types in strict mode to npm, a la https://github.com/protocolbuffers/protobuf/pull/8955