Thanks!

This increases the size of SDK:

Before:

$ esbuild src/browser.protobuf.ts --bundle --minify --sourcemap --outfile=dist/centrifuge.protobuf.js

  dist/centrifuge.protobuf.js      123.2kb
  dist/centrifuge.protobuf.js.map  550.5kb

After:

$ esbuild src/browser.protobuf.ts --bundle --minify --sourcemap --outfile=dist/centrifuge.protobuf.js

  dist/centrifuge.protobuf.js      150.4kb
  dist/centrifuge.protobuf.js.map  803.9kb

And during yarn build-all I still see warning:

(!) Use of eval is strongly discouraged
https://rollupjs.org/troubleshooting/#avoiding-eval
node_modules/@protobufjs/inquire/index.js
10: function inquire(moduleName) {
11:     try {
12:         var mod = eval("quire".replace(/^/,"re"))(moduleName); // eslint-disable-line no-eval
                      ^
13:         if (mod && (mod.length || Object.keys(mod).length))
14:             return mod;

Do you think it's better than before with these changes? I think the problem of eval somewhere in protobufjs still exists?

File size growth:
Argh, it's quite unfortunate. A long time ago these folks explained how the reflection grows at a larger rate than the json export due to the boiler plate.
Then this person wrote a "sanitizer" to remove the boiler plate and unused types.
It took me some digging but I found there exists a --filter and --sparse. Let me give it a try and see what it does to the bundle size!

Eval usage:
Strange ... protobuf js in the CLI description does promise us no eval usage in the minimal version...

Thanks @wrangelvid ! Please let me know if you find sth, will try to look closer too when exactly eval is used in protobufjs, now it's not very clear to me whether current implementation of centrifuge-js uses it or not in practice, or rollup complains just because eval is found in protobufjs source code.

I made an interesting discovery. What do we care more about? The minified file size or the gzip size that actually goes over network?

When we run gzip -c dist/centrifuge.protobuf.js | wc -c | awk '{printf "%.1f KB\n", $1/1024}' on the before and after version:

I made an interesting discovery

LOL, very nice :) I think gzipped is more important.

Also ran benchmark and this version shows better encoding speed: 4.90 µs/op vs 6.19 µs/op in current implementation.

Will allocate more time to look at code generation and hopefully good to go 🤞

Hello @wrangelvid

So I looked at changes, and tested how it works with examples - everything seems good.

While testing I found an issue with current bench suite - it tried to decode encoded commands, not replies.

New bench results with applied fix on top of master and this PR:

Before:

Protobuf Encode: 10000 runs — Total: 59.44 ms, Avg: 5.94 µs/op
Protobuf Decode: 10000 runs — Total: 25.42 ms, Avg: 2.54 µs/op

With this PR:

Protobuf Encode: 10000 runs — Total: 48.02 ms, Avg: 4.80 µs/op
Protobuf Decode: 10000 runs — Total: 18.74 ms, Avg: 1.87 µs/op

So it also improves decoding.

Also looked at source code generated by esbuild, it still includes:

var mod = eval("quire".replace(/^/, "re"))(moduleName);

Not sure maybe it's just can't be tree-shaked, maybe required for something. I think I may try to go with solution in protobufjs/protobuf.js#2057 (comment) - override dependency to inquire with patch from protobufjs/protobuf.js#1941 applied. This may help to avoid eval in browser builds, though users who consume centrifuge from npm still need to apply same overrides in their local package.json from what I understand. But a separate story.

The issues surrounding @protobufjs/inquire make the eval story quite frustrating, but I’m glad someone else cut a release of the subpackage!

Agree.. Strange situation without enough attention from maintainers.

Released https://github.com/centrifugal/centrifuge-js/releases/tag/5.4.0 with this PR included, let's see how it goes. Many thanks for contributing!