Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Google Closure Compiler (Refactor to remove stringified reference to types) #715

Closed
joscha opened this issue Mar 23, 2017 · 4 comments

Comments

@joscha
Copy link

joscha commented Mar 23, 2017

protobuf.js version: 6.6.5 (but also on master)

When generating code, there is this piece:

        const $types = {
            3: "xyz.MyEnum"
        }; $lazyTypes.push($types);

which is then resolved later in util.lazyResolve. I am trying to use protobuf.js in the Things Network Console which allows you to specify a payload function to decode incoming data from a byte buffer. This code is run within otto, so I am trying to get a statically generated decoder as small as possible. With Uglify I get to 22KB for a decode/fromObject/toObject combo (17.4 for decode-only). However, the Google Closure Compiler is far better at getting this even smaller with advanced optimizations (about 12KB). One problem with that, however, is that it renames symbols quite rigidly, so xyz.MyEnum would be renamed and when the lazy loader tries to find it, it won't be there any more.
There are a couple of possible solutions I can see for this. We could either define the types as callbacks, e.g.

        const $types = {
            3: () => xyz.MyEnum
        }; $lazyTypes.push($types);

or we can define them with array notation:

xyz["MyEnum"]= (function() {...}

to prevent them from being modified. Is there a reason why on a static build these are lazily evaluated at all?

@dcodeIO
Copy link
Member

dcodeIO commented Mar 23, 2017

Is there a reason why on a static build these are lazily evaluated at all?

The $root object is assembled linearly in static code, hence a type reference from message 1, which is declared first, to message 2 might not yet be there (undefined or a TypeError if even the parent namespace isn't there yet).

We could either define the types as callbacks

I like this approach.

Edit: Alternatively, access to types[N] in generated functions could even be replaced with direct references.

@dcodeIO
Copy link
Member

dcodeIO commented Mar 23, 2017

This commit removes the need for lazy types entirely. Let me know if there are any issues.

@joscha
Copy link
Author

joscha commented Mar 23, 2017

🎉 @dcodeIO - will check it out later and let you know! Thank you very much!

@joscha
Copy link
Author

joscha commented Mar 24, 2017

works a charm @dcodeIO, just tried with 8401a47 - thank you so much, any chance we can get a new version released?

@joscha joscha closed this as completed Mar 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants