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

disable sorting #8

Closed
ajoslin103 opened this issue Aug 5, 2016 · 4 comments
Closed

disable sorting #8

ajoslin103 opened this issue Aug 5, 2016 · 4 comments

Comments

@ajoslin103
Copy link

ajoslin103 commented Aug 5, 2016

I want to use this library to read already written binary files, porting in c/c++ structs -- but it's sorting the schema and that is totally screwing things up

Please consider an option to disable the schema sorting!!

@phretaddin
Copy link
Owner

phretaddin commented Aug 5, 2016

The keys are sorted on this line: https://github.com/phretaddin/schemapack/blob/master/schemapack.js#L265

The reason schemas are sorted is because iteration order of objects in JavaScript is not guaranteed. For example, if you have the same exact schema on the client and server. It's possible (although not generally likely) that it could be iterated in a different order on both the client and server, which would cause the wrong object to be decoded.

For this reason, the schemas are sorted by alphabetical order and turned in to an array to guarantee traversal order.

If you don't need this, then you can just comment out that line I posted at the top.

@ajoslin103
Copy link
Author

I submitted a pull-request with a change to make it a selectable option -- I didn't think of the iteration order... So the fact that my test worked when only decoding packed data was essentially luck, eh? Hrmmm....

So if one were to do un-sorted one would have to also include an array of fields to specify their order... That would have to prefix the data, and would be a breaking change as you don't enforce a version # at the head of the data-block I think...

@phretaddin
Copy link
Owner

Yeah, just luck. Most of the time it works (especially with newer browsers), but you can run in to cases where the iteration order is different, so sorting or something has to be done first.

Yeah, prefixing the data would be a breaking change. I tried to think of an alternative to alphabetical sorting but didn't come up with much.

@ajoslin103
Copy link
Author

ok, so reject my pull-request and give some thought to my suggestion at the end of the fixed-length elements issue.

thx

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