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

Generic event types #22

Merged
merged 4 commits into from
Dec 3, 2018
Merged

Generic event types #22

merged 4 commits into from
Dec 3, 2018

Conversation

bodil
Copy link
Owner

@bodil bodil commented Dec 1, 2018

This decouples the stdweb bits from the default build, and allows the output type to specify the type of the events blob, which means that events are no longer tied to stdweb types. It moves stdweb behind a feature flag and opens up for alternative implementations, most urgently web-sys.

The drawback: because of a shortcoming with Rust's type inference, the event type can't be inferred by a type annotation external to the macro's generated code, so when event handlers are in use, it's necessary to explicitly tell the macro about the output type: html!(<button onclick="lol()"/>) now needs to be html!(<button onclick="lol()"/> : String), or the compiler will complain that you need an explicit type annotation.

I'm not very happy about this, hence the pull request rather than pushing directly to master.

Unfortunately, this means the macro now requires a type annotation when 
using event handlers.

Closes #6, #17.
@typesanitizer
Copy link

typesanitizer commented Dec 1, 2018

I looked at one of the examples, where the html! macro needs to be passed Stdweb. However, you already know that type using the feature flags. Would it be possible to use conditional compilation to get those types, so users don't have to annotate them? (Pseudocode below, not entirely sure if this would work.)

#[cfg(...)]
type HtmlMacroT1 = Stdweb
#[cfg(...)]
type HtmlMacroT1 = WebSys

macro html! {
    // new annotation to help the compiler!
    let x : HtmlMacroT1 = ...
}

(I'm assuming that the flags are expected to be mutually exclusive, not sure if that is a valid assumption.)

@bodil
Copy link
Owner Author

bodil commented Dec 1, 2018

I thought about this, but I'm not quite sure what the best solution would be. Ideally it should be user configurable by the consumer, given that they may be using an output type that isn't provided by the base crate. I think a best effort choice of defaulting to String vs Stdweb vs whatever other feature flags that may appear is probably better than nothing, though.

@typesanitizer
Copy link

typesanitizer commented Dec 1, 2018

If you want full flexibility, will users want the same types everywhere, or would they want very fine-grained flexibility? If it is the former, then you can offer a functorized interface (i.e. like ML functors) using a higher order macro.

macro make_default_typed_html_macros
    #[cfg(...)]
    type T1 = ...
    type T2 = ...
    #[cfg(otherwise)]
    type T1 = ...
    type T2 = ...
    make_typed_html_macros! { type T1 = ...;  type T2 = ...; }

macro make_typed_html_macros
    // read T1, T2 ... from supplied arguments
    macro html
        let x1 : T1 = ...

This reduces the annotation burden to 1 place in the code base, as opposed to every place you use a macro.

Of course, this comes with its own set of problems, and doesn't scale (e.g. compile time) if you want fine-grained flexibility 🤷‍♂️

@bodil
Copy link
Owner Author

bodil commented Dec 3, 2018

Actually, that might work really well. I'm going to attempt an implementation.

@bodil
Copy link
Owner Author

bodil commented Dec 3, 2018

Seems to be impossible - I can't find a way to declare a macro with a nested macro that uses repeat patterns (rust-lang/rust#35853 (comment) looked promising, but ultimately produces unexpected errors where a directly declared wrapper macro doesn't), and I can't do it in a proc macro because proc macros with nested macros require #![feature(proc_macro_hygiene)] and it seems a shame to revert to the nightly requirement. I could introduce a more awkward syntax to avoid the repeat pattern, but I think I prefer the required type annotation.

@bodil bodil merged commit 4c49aca into master Dec 3, 2018
@bodil bodil deleted the generic-events branch December 3, 2018 17:03
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 this pull request may close these issues.

2 participants