Can't compile egui_extras without serde feature

[hacknus]

I tried the new version of egui (0.28) but egui-extras seems to fail compiling with the following messages:

in `synthax_highlighting.rs

error[E0277]: the trait bound `CodeTheme: SerializableAny` is not satisfied
   --> /Users/linus/.cargo/registry/src/index.crates.io-6f17d22bba15001f/egui_extras-0.28.0/src/syntax_highlighting.rs:158:19
    |
158 |                 d.get_persisted(egui::Id::new("dark"))
    |                   ^^^^^^^^^^^^^ the trait `serde::ser::Serialize` is not implemented for `CodeTheme`, which is required by `CodeTheme: SerializableAny`
    |
    = note: required for `CodeTheme` to implement `SerializableAny`
note: required by a bound in `IdTypeMap::get_persisted`
   --> /Users/linus/.cargo/registry/src/index.crates.io-6f17d22bba15001f/egui-0.28.0/src/util/id_type_map.rs:397:29
    |
397 |     pub fn get_persisted<T: SerializableAny>(&mut self, id: Id) -> Option<T> {
    |                             ^^^^^^^^^^^^^^^ required by this bound in `IdTypeMap::get_persisted`

and in table.rs:

error[E0277]: the trait bound `TableState: SerializableAny` is not satisfied
   --> /Users/linus/.cargo/registry/src/index.crates.io-6f17d22bba15001f/egui_extras-0.28.0/src/table.rs:547:45
    |
547 |             .data_mut(|d| d.get_persisted::<Self>(state_id))
    |                             -------------   ^^^^ the trait `serde::ser::Serialize` is not implemented for `TableState`, which is required by `TableState: SerializableAny`
    |                             |
    |                             required by a bound introduced by this call
    |
    = note: required for `TableState` to implement `SerializableAny`
note: required by a bound in `IdTypeMap::get_persisted`
   --> /Users/linus/.cargo/registry/src/index.crates.io-6f17d22bba15001f/egui-0.28.0/src/util/id_type_map.rs:397:29
    |
397 |     pub fn get_persisted<T: SerializableAny>(&mut self, id: Id) -> Option<T> {
    |                             ^^^^^^^^^^^^^^^ required by this bound in `IdTypeMap::get_persisted`

My cargo.toml looks like this:

[dependencies]  
eframe = { version = "0.28", features = ["persistence"] }  
egui_extras = { version = "0.28" }  
egui_plot = "0.28"  
egui-phosphor = { git = "https://github.com/hacknus/egui-phosphor" }  

And I'm using it on macOS 14.5.

egui-phosphor is bumped to egui 0.28 and works fine. the issue lies in egui_extras.

[lukexor]

This seems to be an accidentally forced dependency on the "serde" feature. Enabling it resolves the issue, but it's supposed to be optional.

[crumblingstatue]

Looks like the current table implementation just assumes that the serde feature is enabled. There are unconditional calls to TableState::load for example in TableBuilder::header. And TableState::load unconditionally tries to call get_persisted.

[crumblingstatue]

Workaround until this is fixed:

egui_extras = { version = "0.28.0", features = ["serde"] }

Adding this to Cargo.toml even works if it's not a direct dependency (required by egui_commonmark for example).

[emilk]

We should:
A) set up a CI step to catch this problem
B) only call get_persisted if the serde feature is enabled

[YgorSouza]

The problem is actually in egui, not egui_extras. It happens if we try to build egui_extras without the serde feature while something else enabled the persistence feature in egui. This was previously not possible because serde was a mandatory dependency in egui_extras.

cargo build -p egui_extras --features egui/persistence

egui/crates/egui/src/util/id_type_map.rs

Lines 38 to 54 in b31d02d

	#[cfg(feature = "persistence")]
	pub trait SerializableAny:
	'static + Any + Clone + serde::Serialize + for<'a> serde::Deserialize<'a> + Send + Sync
	{
	}
	#[cfg(feature = "persistence")]
	impl<T> SerializableAny for T where
	T: 'static + Any + Clone + serde::Serialize + for<'a> serde::Deserialize<'a> + Send + Sync
	{
	}
	#[cfg(not(feature = "persistence"))]
	pub trait SerializableAny: 'static + Any + Clone + for<'a> Send + Sync {}
	#[cfg(not(feature = "persistence"))]
	impl<T> SerializableAny for T where T: 'static + Any + Clone + for<'a> Send + Sync {}

Enabling the feature causes the trait bounds to change in a semver breaking way, which should not happen because features are supposed to be additive. I don't know if there is a way to avoid this, though, aside from making serde mandatory in egui.

[emilk]

Good catch… yeah, if persistence is on for egui and serde is off for egui_extras, we will have this problem. Not easily solved if we want serde to be optional for egui_extras.

As a short-term solution I can make serde a default (opt-out) feature of egui_extras, so at least fewer people get bitten by this.

[torokati44]

Good catch… yeah, if persistence is on for egui and serde is off for egui_extras, we will have this problem.
As a short-term solution I can make serde a default (opt-out) feature of egui_extras, so at least fewer people get bitten by this.

Wouldn't making the persistence feature depend on egui_extras/serde also have worked? 😶 It would have created a bit less headache IMHO, and maybe it could even have been considered a "proper" solution, not just a short term one? 🤔 Of course it's entirely possible that I'm just missing something that makes this not quite as simple...

[emilk]

egui/persistence cannot depend on egui_extras/serde, because egui_extras depends on egui

[torokati44]

Ah, right, this is what I missed...

[emilk]

We should be able to fix this by only calling get_persisted/insert_persisted if #[cfg(feature = "serde")], and otherwise call get_temp/insert_temp

[aurexav]

Is anyone working on this? I'm eagerly anticipating the new release.

[YgorSouza]

We should be able to fix this by only calling get_persisted/insert_persisted if #[cfg(feature = "serde")], and otherwise call get_temp/insert_temp

If this is what has to be done anyway, then maybe get_persisted/insert_persisted in egui should be conditional on the persistence feature instead of having different trait bounds? That means each crate that uses egui would have to implement its own logic to call get_persisted or get_temp depending on its own enabled features.

This would be in accordance with the recommendation on https://doc.rust-lang.org/cargo/reference/features.html#feature-unification

A consequence of this is that features should be additive. That is, enabling a feature should not disable functionality, and it should usually be safe to enable any combination of features. A feature should not introduce a SemVer-incompatible change.

[aurexav]

vergen recently made that change.

https://github.com/rustyhorde/vergen?tab=readme-ov-file#%EF%B8%8F-notes-on-version-9-%EF%B8%8F

[emilk]

If this is what has to be done anyway, then maybe get_persisted/insert_persisted in egui should be conditional on the persistence feature instead of having different trait bounds? That means each crate that uses egui would have to implement its own logic to call get_persisted or get_temp depending on its own enabled features.

The issue at hand occurs when egui:persistence is ON and egui_extras:serde is OFF.

[torokati44]

Oh, was this the last open issue for the "Next Patch Release" milestone? 👀 ^^