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

Control flow in template! #18

Closed
lukechu10 opened this issue Mar 8, 2021 · 3 comments · Fixed by #36
Closed

Control flow in template! #18

lukechu10 opened this issue Mar 8, 2021 · 3 comments · Fixed by #36
Labels
A-macro Area: macros A-reactivity Area: reactivity and state handling C-enhancement Category: new feature or improvement to existing feature

Comments

@lukechu10
Copy link
Member

Implement control flow constructs (if/else and for) in template! macro.

@lukechu10 lukechu10 added C-enhancement Category: new feature or improvement to existing feature A-macro Area: macros labels Mar 8, 2021
@lukechu10 lukechu10 modified the milestone: 0.2 Mar 8, 2021
@TheRawMeatball
Copy link

Hi, I just saw this project and I really like it. I tried to implement this by implementing nested effects, but it was much harder than expected and I couldn't get it to work. Do you have a better idea for how this can be accomplished?

@lukechu10 lukechu10 added S-blocked Waiting on another issue/PR and removed S-blocked Waiting on another issue/PR labels Mar 9, 2021
@lukechu10
Copy link
Member Author

Hi, I just saw this project and I really like it. I tried to implement this by implementing nested effects, but it was much harder than expected and I couldn't get it to work. Do you have a better idea for how this can be accomplished?

First off, for the design, there are two potential ways to implement this. The most obvious one would be to introduce if/else and for constructs directly in the template! macro. The second one would be to use components for control flow (which is actually how solid-js does it: https://github.com/ryansolid/solid/blob/master/documentation/rendering.md).

I am personally in favor of the first option, that is, adding control flow directly to template! because it looks cleaner in the markup and it is more obvious that it is control flow, not components.

Arguments for favoring the second option, that is, using components, would include a simpler and smaller API. We would just need to build these components.

In any case, for the actual logic for rendering templates with control flow, it should just use create_effect to evaluate the condition and than render the appropriate template. For example, this could be what an if/else construct expands to with template!:

template! {
    @if condition() 
        p { # "true" }
    } else {
        p { # "false" }
    }
}

// expands to

{
    let node = /* unintialized_placeholder */;
    create_effect(move || {
        if condition() {
            let element = create_element("p");
            append(&element, text("true");
            element
        } else {
            let element = create_element("p");
            append(&element, text("false");
            element
        }
    });

    node.unwrap()
}

@lukechu10
Copy link
Member Author

lukechu10 commented Mar 9, 2021

@TheRawMeatball I saw you're work here TheRawMeatball@f1a4ede and new code was pushed over in #20 so you might want to try rebasing. I think it would be better if you created a PR for nested effects first, if you want of course. If not, let me know and I'll be happy to do it.

About that, I think the easiest way to implement it would simply be to store DEPENDENCIES in a temporary, let's call it outer_dependencies using RefCell::take, replacing these lines:
https://github.com/lukechu10/maple/blob/5b893e384587381d6449d1d344617f7b801f514d/maple-core/src/reactive.rs#L178-L180

Over here, we would need to set outer_dependencies back to DEPENDENCIES, basically replacing the None value in the source with outer_dependencies.
https://github.com/lukechu10/maple/blob/5b893e384587381d6449d1d344617f7b801f514d/maple-core/src/reactive.rs#L192-L193

I believe this would have the correct behavior, which is dependencies in the inner effect should not also be dependencies of the outer effect.

@lukechu10 lukechu10 added S-blocked Waiting on another issue/PR A-reactivity Area: reactivity and state handling labels Mar 9, 2021
@lukechu10 lukechu10 mentioned this issue Mar 9, 2021
3 tasks
@lukechu10 lukechu10 removed the S-blocked Waiting on another issue/PR label Mar 10, 2021
@lukechu10 lukechu10 reopened this Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macro Area: macros A-reactivity Area: reactivity and state handling C-enhancement Category: new feature or improvement to existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants