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

Overriding util.fetch to load files from zip bundle #684

Closed
delapavap opened this issue Feb 20, 2017 · 8 comments
Closed

Overriding util.fetch to load files from zip bundle #684

delapavap opened this issue Feb 20, 2017 · 8 comments
Labels

Comments

@delapavap
Copy link

delapavap commented Feb 20, 2017

protobuf.js version: 6.3.3
Hi, This is a Question not an issue..., I'm trying to create a monkey patch to override util.fetch function because I need to load .proto files from a zip bundle. the thing is, I have a .proto with two imports (3 files to load in total) so.. When I check root object after load (promise way), the file's array contains all 3 files which means they are loaded correctly, but when I check all the way through nested objects I can figure out that last imported file wasn't processed. I'm using this function to override util.fetch

    BundleCache.prototype.fetchFromSimpleBundle = function (zip) {
            return function fetch(filename, options, callback) {
                try {
                    if (typeof options === "function") {
                        callback = options;
                        options = {};
                    } else if (!options)
                        options = {};

                    var proto = zip.file(filename);
                    var content = proto ? proto.asBinary() : null;
                    callback(null, content);
                } catch (error) {
                    callback(error);
                }
            };
        };

what am I missing or doing wrong?

I'm using nodeJs 4.6.0 btw.

@dcodeIO
Copy link
Member

dcodeIO commented Feb 20, 2017

Looks like your fetch implementation is synchronous, which might cause issues. Try to wrap the callback invocations in a setImmediate / setTimeout or something, and let me know.

Also, make sure to either honor options.binary, which makes the result a buffer/uint8array if binary=true or otherwise a string, or, if you are not loading binary at all, just make sure that your implementation calls the callback with a string.

Fetch reference

@delapavap
Copy link
Author

@dcodeIO Thanks! it worked perfectly, another question... is it posible that the decoded object use snake_case instead of camelCase? we have a lot of plugins that references properties using snake_case.

Again, thank you for your help

@dcodeIO
Copy link
Member

dcodeIO commented Feb 22, 2017

Root#load takes an option to do that, like so:

var root = new protobuf.Root();
root.load("myfile.proto", { keepCase: true }).then(...);

@delapavap
Copy link
Author

delapavap commented Mar 8, 2017

@dcodeIO again, thank you for your help, keepCase: true works only using the callback function, when you use load as a promise it will set options as undefined because in the next call the second argument is a function :

Root.prototype.load = function load(filename, options, callback) {
if (typeof options === "function") {
        callback = options;
        options = undefined;
    }
...

So, I think there's a bug.

I want to ask you another thing, I need to add a custom property to every object just for visualization (e.g namespace + type), but the message has its own toJSON methods. therefore, my custom property is not added. what should I do ? I don't wanna use JSON.parse(JSON.stringify(decoded)); because it will hit performance (we have very big objects in some scenarios)

@dcodeIO
Copy link
Member

dcodeIO commented Mar 8, 2017

Regarding your question: You could override the default toJSON implementation:

protobuf.Message.prototype.toJSON = (function(toJSON_default) {
  return function toJSON_ex() {
    var res = toJSON_default.call(this);
    res._ns = this.$type.fullName; // or any other custom property
    return res;
  };
})(protobuf.Message.prototype.toJSON);

@delapavap
Copy link
Author

Hi @dcodeIO it's me again with another question. We have an index out of range error with some messages and I think it's because this : option (scalapb.message).extends = "MySuperClass";
As far as I have searched this is some kind of customization of scalapb (Protocol Buffer Compiler for Scala) but I'm not pretty sure... perhaps you know something about this or an equivalent way of declaring this in a standard way.

Thank you

@dcodeIO
Copy link
Member

dcodeIO commented Mar 14, 2017

Yeah, that looks like something custom. I don't know how they handle it, but you could try to simply copy the fields of whatever it extends to the message with the option - basically join it, if that's what they do.

If that's it, then this could oft course be done automatically by using reflection.

@dcodeIO
Copy link
Member

dcodeIO commented Mar 22, 2017

Not sure if this issue has already been solved or not. Closing it for now. Feel free to reopen if there are any remaining issues!

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

No branches or pull requests

2 participants