-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Switch const serialize to use CBOR #4932
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
base: main
Are you sure you want to change the base?
Conversation
|
I wonder if we might be able to add a header with a magic number to identify different versions from each other, letting us support the old format and the new cbor format both in 0.7 |
Supporting the old version of assets in the new version of the CLI is fairly easy. The difficult case is supporting the new version of assets in the old version of the CLI. If you run |
We could push an update that reads the current |
|
We could, wasm-bindgen does something similar requiring a matching version of the wasm-bindgen-cli and wasm-bindgen itself. That has made it difficult to work with wasm bindgen programmatically in our CLI. We now bundle a wasm-bindgen version management system within the CLI I imagine consumers of |
| repository = "https://github.com/dioxuslabs/dioxus" | ||
| homepage = "https://dioxuslabs.com/learn/0.5/getting_started" | ||
| keywords = ["const", "serialize"] | ||
| rust-version = "1.80.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note the rust version bump. If I remember correctly, we support 3 versions behind stable which would be 1.88 (1.91.1 - 3). 1.83 includes support for &mut T in const which makes this code a lot nicer to maintain
I changed this without realizing we published 0.7 with an old rust-version requirement. Cargo is a bit smarter when updating now so it won't update to the new version if our rust-version is newer than the currently installed rust version. That said, this may break code if you are using an old version of cargo so I may end up reverting this change
|
The CLI now decodes both the 0.7.0-0.7.1 version of assets (under the While this is still breaking for const-serialize itself, it shouldn't be breaking for manganis or dioxus since neither of those crates export const serialize types outside of unstable macro helpers |
Exciting! Will give it a test drive. Very keen to start adding more variants for things like serverfn metadata and it will be useful to do this without requiring a full version bump |
Currently const serialize uses a bespoke endianness erased packed format which is very simple to support, but is not self describing. This makes any change to the layout of the data including adding fields or adding enum variants a breaking change.
This PR switches to CBOR which is both a relatively simple format to support and is self describing.
While this PR should make breaking changes to manganis more forgiving in the future, the change in format itself is breaking because old version of the CLI don't know how to deserialize the new cbor format
TODO:
Closes #4863