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

Internals change breaks pre-generated file #649

Closed
jkrems opened this issue Jan 17, 2017 · 4 comments
Closed

Internals change breaks pre-generated file #649

jkrems opened this issue Jan 17, 2017 · 4 comments

Comments

@jkrems
Copy link

jkrems commented Jan 17, 2017

protobuf.js version: protobufjs@6.5.0

The following code has been generated by a previous version (I think 6.4.5):

                        RecordSet.from = function from(source, options) {
                            return this.convert(source, $protobuf.converters.message, options);
                        };

Running with the latest version of protobufjs, I get the following error:

Uncaught TypeError: Cannot read property 'message' of undefined
@jkrems
Copy link
Author

jkrems commented Jan 17, 2017

Looking at the changelog - should we be pinning to a specific version of the runtime or at least to a specific minor?

@dcodeIO
Copy link
Member

dcodeIO commented Jan 17, 2017

Static code is still somewhat in flux due to being an entirely new feature. Hence, compatibility between minor version updates (i.e. 6.4 to 6.5) can't really be guaranteed. In such a case I'd recommend to depend on "protobuf.js": "~6.5.0" (or likewise) for now, which only accepts patch updates, or to take possibly re-generations of the code into account.

In the case of 6.4 to 6.5, converters have been upgraded to first class codegen citizens. While from works pretty much the same as before, it doesn't use a general converter (here: this.convert) anymore but uses specialized methods for each.

@jkrems
Copy link
Author

jkrems commented Jan 17, 2017

Cool, thanks. We'll investigate moving code generation to startup and for now we'll narrow it down to ~6.5.0.

@jkrems jkrems closed this as completed Jan 17, 2017
@dcodeIO
Copy link
Member

dcodeIO commented Jan 17, 2017

Also going to think about semver-compatible versioning again with the next breaking change. If you see a protobuf.js v7 in the near future, you'll know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants