Has json-module with reflection changed since 6.8?

[hnfmr]

protobuf.js version: 6.8

I have been using protobuf.js to generate json-module with reflection information, it's been working fine until 6.7.3 and a recent upgrade to 6.8 is throwing errors.

For example I use the following command to generate json-module:

pbjs --target json-module -w commonjs --keep-case -o dest.js source.proto

The errors:

$ mocha test/protobuf.spec.js
/Users/xxx/xxx/node_modules/xxx/xxxx/dest.js:6
var $root = ($protobuf.roots["default"] || ($protobuf.roots["default"] = new $protobuf.Root()))
                                                                         ^

TypeError: $protobuf.Root is not a constructor
    at Object.<anonymous> (/Users/xxx/xxx/node_modules/xxx/xxxx/dest.js:6:74)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)

Thanks!!

[dcodeIO]

There have been a couple of changes along the road, but without additional context from your generated file it's hard to tell. roots and Root should be there. What's the require statement prior to your snippet? Looks like it doesn't require protobuf.js.

[hnfmr]

Wow thanks for the prompt reply.

Here is the header part of the generated dest.js using protobuf.js 6.8:

/*eslint-disable block-scoped-var, no-redeclare, no-control-regex, no-prototype-builtins*/
"use strict";

var $protobuf = require("protobufjs/minimal");

var $root = ($protobuf.roots["default"] || ($protobuf.roots["default"] = new $protobuf.Root()))
.addJSON({
  pb: {
    nested: {

Here is the same header part generated using protobufjs 6.7.3:

/*eslint-disable block-scoped-var, no-redeclare, no-control-regex, no-prototype-builtins*/
"use strict";

var $protobuf = require("protobufjs");

var $root = ($protobuf.roots["default"] || ($protobuf.roots["default"] = new $protobuf.Root()))
.addJSON({
  pb: {
    nested: {

[dcodeIO]

Ah, for some reason this is now including the minimal library, which doesn't include reflection at all (so no Root).

Could be related to #813

Ideally, it should be require("protobufjs/light"); at this place as the parser isn't required when using JSON modules.

[hnfmr]

Yep, a comparison made it clear. The protobufjs/minimal is probably the cause. Thanks a lot!

[hnfmr]

Also that I have the following errors if I change the require("protobufjs/minimal") to
require("protobufjs/light") or just require("protobufjs"):

Suppose I have a type from reflection lookup:

const CustomT = root.lookup('pb.SomeCustomType')

It complains CustomT.from is not a function, and if I change from to create, it works, but it then complains that toObject is not a function:

const a = CustomT.from(other);
const decodedMessage = CustomT.decode(buffer);
decodedMessage.toObject({defaults: true});

[dcodeIO]

from has been dropped in favor of fromObject and both this one and toObject are solely static methods now.

const a = CustomT.fromObject(other);
const decodedMessage = CustomT.decode(buffer);
CustomT.toObject(decodedMessage , {defaults: true});

[hnfmr]

Thanks for the quick update!

[gilly3]

from has been dropped in favor of fromObject and both this one and toObject are solely static methods now.

When you remove methods from your public API, that is a breaking change, and warrants a major version bump. The current version should be 7.0.0, not 6.8.0. It would be nice to give a release or two with a deprecation notice before removing methods.

[dcodeIO]

See also, note at https://github.com/dcodeIO/protobuf.js#nodejs