Open dcodeIO opened 4 years ago
On a more general note, a good path forward appears to be to merge https://github.com/protobufjs/protobuf.js/pull/1356 soonish, and continue from there by applying the new release processes, versioning etc. that have been set up?
@dcodeIO Looking at #1356.
@dcodeIO #1356 is good to go! Then we can complete the release automation (Cc: @bcoe) and keep moving.
As for the list of things to work on, #1234 is also high in my personal priority list (since it will make it possible to use CLI tools in hermetic environments like bazel, and we do need it).
Thank you for forwarding the email request, I will do my best here :)
Merged that PR meanwhile, thanks for fixing! Shall we make a 6.9.0 release with the new process next?
Shall we make a 6.9.0 release with the new process next?
This sounds great to me, @alexander-fenster let me know if you need any help rolling out today?
@dcodeIO @alexander-fenster we rolled out the first release using the new process this morning 🎉
Shall we follow up with the folks emailing you?
catching up, sounds like we'll also need to work on reviewing and merging. #1256? perhaps we can let our new release bake for a bit, and then prioritize landing that fix?
- Another party has made changes to code generation, in that it splits emitted files into one per service / message / enum to reduce overhead on the frontend. They are asking if there is interest in this, if they open sourced it?
No. Strictly speaking, protobuf.js is a generator, and not a compiler.
Dead code elimination is a classical task for a compiler. And JavaScript is no exception to this rule. Closure Compiler e.g. parses JavaScript, analyzes it, removes dead code and rewrites and minimizes what's left. This is common best practice.
ok
@cherryland If I'm reading the original request correctly, it's just about splitting the code, not to do any optimization or dead code removal. If we run pbjs
today, we get a huge .js
file, and I can imagine why it could make sense to split it by services, messages, or anything else.
(as an example, this generated js file from one of our libraries weighs 1.1MB, and it's not the biggest one I saw)
@alexander-fenster You are right brother, on JavaScript Island 🏝️ the traditional approach to reduce file size is mere code splitting and nothing else. This was acceptable when Netscape was a thing and no other tools existed. I understand that.
The output from protobuf.js is meant to be consumed as a library. A developer makes the necessary references to the output from protobuf.js and defines the chunks (or code splits) at a higher level. A JavaScript compiler goes ahead and strips the unused parts based upon the dependency graph of the chunk. And as a developer, you always have the option to build a second proto file.
@cherryland hi, my team at Dropbox added the no-bundle
feature, and maintains a fork of protobufjs (https://github.com/dropbox/protobuf.js) for this and other reasons we have PRs open for.
I'm not sure if what @dcodeIO mentioned in bullet 2 is this feature or something else (sounds similar but might not actually be the same), but if it is I think you may have a misunderstanding. The no-bundle flag isn't about eliminating dead code, it's about eliminating duplicate code.
pbjs generates a single bundle of JS for all of the transitive .protos. Therefore if you run pbjs on proto A and then proto B, and both depend on C, then you'll end up with two copies of C's generated JS code -- a.js will contain it, as well as b.js. This is a problem if you need to load proto A and B on the same page for two reasons:
code bloat - you're loading C's code twice. If you need to load many protos and they share dependencies, you'll end up with an explosion of duplicate code.
orphaned object references - because of the way protobuf creates the roots data structure at runtime, every time C is loaded into memory it replaces the previous instance of C. Therefore you can potentially end up with references to different instances of C. Practically I'm not sure what issues this could cause but best to be avoided.
To work around this you could use pbjs to generate one bundle of JS bundle per page, but this isn't ideal for two reasons:
You could also generate one bundle for all pages, but this does not scale.
The fact that pbjs bundles at all is a little weird. The protoc tool doesn't do this for any language, including JS. And even for web browsers this isn't ideal for the reasons stated. If bundling is needed, asset bundlers (e.g. rollup / webpack) should be used for this.
Example
my/protos/c.proto
package my_protos_c;
message C {
string some_field = 1;
}
my/protos/a.proto
package my_protos_a;
import "my/protos/c.proto";
message A {
my_proto_c.C nested_msg_c = 1;
}
my/protos/b.proto
package my_protos_b;
import "my/protos/c.proto";
message B {
my_proto_c.C nested_msg_c = 1;
}
Strategy 1: run pbjs on each proto
pbjs --target static-module --out a.js a.proto
pbjs --target static-module --out b.js b.proto
import {my_protos_a} from 'my/protos/a';
import {my_protos_b} from 'my/protos/b';
page loads C's code twice
Strategy 2: generate one js bundle per page with pbjs
pbjs --target static-module --out page_1.js a.proto
pbjs --target static-module --out page_2.js b.proto
page 1
import {my_protos_a} from 'my/protos/page_1';
page 2
import {my_protos_b} from 'my/protos/page_2';
page 1 loads different copy of C than page 2
Strategy 3: generate one .js file for every .proto
pbjs --target static-module --path out/ --no-bundle a.proto
pbjs --target static-module --path out/ --no-bundle b.proto
pbjs --target static-module --path out/ --no-bundle c.proto
import {my_protos_a} from 'my/protos/a';
import {my_protos_b} from 'my/protos/b';
only 1 copy of C is loaded
page 1
import {my_protos_a} from 'my/protos/page_1';
page 2
import {my_protos_b} from 'my/protos/page_2';
page 1 loads same copy of C as page 2
@dcodeIO Very large interest in the second bullet point. I just started to look into how to do this for my company.
I've received two mails about protobuf.js today, which is quite an uncommon thing to happen nowadays. Thought I forward these here so everyone, especially the maintainers who joined, is informed. Here is a summary:
Interestingly, I also had one email about bytebuffer.js:
getInt
forreadInt
to make it work more like Java's ByteBuffer class?