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

Comments

Projects
None yet
2 participants
@delapavap

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

This comment has been minimized.

Owner

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

@dcodeIO dcodeIO added the question label Feb 20, 2017

@delapavap

This comment has been minimized.

delapavap commented Feb 21, 2017

@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

This comment has been minimized.

Owner

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

This comment has been minimized.

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 added a commit that referenced this issue Mar 8, 2017

@dcodeIO

This comment has been minimized.

Owner

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

This comment has been minimized.

delapavap commented Mar 14, 2017

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

This comment has been minimized.

Owner

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

This comment has been minimized.

Owner

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 Mar 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment