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

xilem_web: Panic using BoxedView inside HTML element #274

Closed
matthunz opened this issue May 5, 2024 · 1 comment · Fixed by #403
Closed

xilem_web: Panic using BoxedView inside HTML element #274

matthunz opened this issue May 5, 2024 · 1 comment · Fixed by #403

Comments

@matthunz
Copy link
Contributor

matthunz commented May 5, 2024

When trying to use a BoxedView in an html::div I get the following panic:

panicked at C:\Users\matth\.cargo\git\checkouts\xilem-031d790027f85a11\1e37164\crates\xilem_web\src\context.rs:304:14:
Element type has changed, this should never happen!

Reproducible by changing the counter example:

fn btn(
    label: &'static str,
    click_fn: impl Fn(&mut AppState, web_sys::MouseEvent) + 'static,
) ->  BoxedView<AppState> {
    Box::new(el::button(label).on_click(click_fn))
}

fn app_logic(state: &mut AppState) -> impl View<AppState> {
    el::div((
        btn("+1 click", |state, _| state.increment()),
        state.text.clone(),
    ))
}
Stack trace
Stack:

Error
    at imports.wbg.__wbg_new_abda76e883ba8a5f (http://localhost:8080/lookbook-xilem-example-57aa3af25ad35cc9.js:338:21)
    at lookbook_xilem_example-f6a974525f70f53f.wasm.console_error_panic_hook::Error::new::h516a79c7e649e619 (http://localhost:8080/lookbook-xilem-example-57aa3af25ad35cc9_bg.wasm:wasm-function[2255]:0xa99ca)
    at lookbook_xilem_example-f6a974525f70f53f.wasm.console_error_panic_hook::hook_impl::h5e4d6d7771b2c70d (http://localhost:8080/lookbook-xilem-example-57aa3af25ad35cc9_bg.wasm:wasm-function[432]:0x55f76)
    at lookbook_xilem_example-f6a974525f70f53f.wasm.console_error_panic_hook::hook::h99fffc916c6dad72 (http://localhost:8080/lookbook-xilem-example-57aa3af25ad35cc9_bg.wasm:wasm-function[3139]:0xb8210)
    at lookbook_xilem_example-f6a974525f70f53f.wasm.core::ops::function::Fn::call::h97eb8c787f0b3f7f (http://localhost:8080/lookbook-xilem-example-57aa3af25ad35cc9_bg.wasm:wasm-function[2707]:0xb2178)
    at lookbook_xilem_example-f6a974525f70f53f.wasm.std::panicking::rust_panic_with_hook::h0833ff9dc94df5f1 (http://localhost:8080/lookbook-xilem-example-57aa3af25ad35cc9_bg.wasm:wasm-function[1002]:0x7eb17)
    at lookbook_xilem_example-f6a974525f70f53f.wasm.std::panicking::begin_panic_handler::{{closure}}::hc3329d7ae0363c6f (http://localhost:8080/lookbook-xilem-example-57aa3af25ad35cc9_bg.wasm:wasm-function[1373]:0x90226)
    at lookbook_xilem_example-f6a974525f70f53f.wasm.std::sys_common::backtrace::__rust_end_short_backtrace::h4268105044501476 (http://localhost:8080/lookbook-xilem-example-57aa3af25ad35cc9_bg.wasm:wasm-function[3596]:0xbc275)
    at lookbook_xilem_example-f6a974525f70f53f.wasm.rust_begin_unwind (http://localhost:8080/lookbook-xilem-example-57aa3af25ad35cc9_bg.wasm:wasm-function[2830]:0xb4058)
    at lookbook_xilem_example-f6a974525f70f53f.wasm.core::panicking::panic_fmt::h765acaeb0182f5af (http://localhost:8080/lookbook-xilem-example-57aa3af25ad35cc9_bg.wasm:wasm-function[2970]:0xb5f83)


imports.wbg.__wbg_error_f851667af71bcfc6 @ lookbook-xilem-example-57aa3af25ad35cc9.js:332

This looks to be related to Pod::downcast_mut, where the underlying element type is incorrect.
I'm also not using any conditionals or a ViewSequence so this is definitely confusing to me, maybe AnyView is storing the wrong type?

@Philipp-M
Copy link
Contributor

Philipp-M commented May 8, 2024

Hmm this is a tricky one, I think it's this issue: https://stackoverflow.com/questions/73807369/cant-downcast-from-mut-boxdyn-trait-as-mut-dyn-any

I.e. in the ViewSequence impl for impl View is coercing the inner element of the Pod which is already of type Box<dyn AnyNode> as &mut dyn Any (via .as_any_mut()) and then back to Box<dyn AnyNode>

github-merge-queue bot pushed a commit that referenced this issue Jun 28, 2024
This ports xilem_web to the new xilem_core.

There's also a lot of cleanup internally:
* Get rid of all of the complex macros to support DOM interfaces, and
instead use associated type bounds on the `View::Element`.
* Introduce an extendable modifier based system, which should also work
on top of memoization (`Arc`, `Memoize`) and `OneOf` views with an
intersection of the modifiable properties.
* This modifier based system gets rid of the hacky way to propagate
attributes to elements, and takes inspiration by masonrys `WidgetMut`
type to apply changes.
* Currently that means `Attributes`, `Classes` and `Styles` to reflect
what xilem_web previously offered.

Downsides (currently, needs some investigation):

~~Due to more internal type complexity via associated types this suffers
from rust-lang/rust#105900. The new trait
solver should hopefully mitigate some of that, but it seems currently it
completely stalls in the todomvc example (not in other examples).~~
~~The deep, possibly completely static composition via associated
type-bounds of the view and element tree unfortunately can take some
time to compile, this gets (already) obvious in the todomvc example. The
other examples don't seem to suffer that bad yet from that issue,
probably because they're quite simple.~~

~~I really hope we can mitigate this mostly, because I think this is the
idiomatic (and more correct) way to implement what the previous API has
offered.~~

One idea is to add a `Box<dyn AnyViewSequence>`, as every element takes
a "type-erased" `ViewSequence` as parameter, so this may solve most of
the issues (at the slight cost of dynamic dispatch/allocations).

Edit: idea was mostly successful, see comment right below.

I think it also closes #274

It's a draft, as there's a lot of changes in xilem_core that should be
upstreamed (and cleaned up) via separate PRs and I would like to
(mostly) fix the slow-compile time issue.

---------

Co-authored-by: Daniel McNab <36049421+DJMcNab@users.noreply.github.com>
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

Successfully merging a pull request may close this issue.

2 participants