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

Update TodoMVC example to use Context API #279

Closed
lukechu10 opened this issue Oct 15, 2021 · 8 comments · Fixed by #295
Closed

Update TodoMVC example to use Context API #279

lukechu10 opened this issue Oct 15, 2021 · 8 comments · Fixed by #295
Assignees
Labels
A-examples Area: examples good first issue Good for newcomers

Comments

@lukechu10
Copy link
Member

The TodoMVC example is a good reference for learning Sycamore. It should be updated to use the context API to prevent "prop-drilling"

@lukechu10 lukechu10 added good first issue Good for newcomers A-examples Area: examples labels Oct 15, 2021
@iiaishwarya
Copy link

I can work on this

@lukechu10
Copy link
Member Author

awesome!

@iiaishwarya iiaishwarya removed their assignment Oct 23, 2021
@iiaishwarya
Copy link

Apologies, I'm removing my assignment as I'm really busy and not able to work on it.

@lukechu10
Copy link
Member Author

No worries

@haywoood
Copy link
Contributor

I'd love to work on this! Just got oriented around the API in my own project 😃

@haywoood
Copy link
Contributor

I'm hitting an issue while refactoring and I created this repo to reproduce the issue using the smallest amount of code. The problem arises with application events that update vectors like the following taken from the linked repo:

    let handle_click = cloned!((state) => move |_event: Event| {
        let new_count: Vec<Signal<i32>> = state.counts
            .get()
            .as_ref()
            .clone()
            .into_iter()
            .chain(Some(Signal::new(0)))
            .collect();
        state.counts.set(new_count)
    });

Which is the same as the code for adding a todo:

    fn add_todo(&self, title: String) {
        let new_todos: Vec<Signal<Todo>> = self.todos
                            .get()
                            .as_ref()
                            .clone()
                            .into_iter()
                            .chain(Some(Signal::new(Todo {
                                title,
                                completed: false,
                                id: Uuid::new_v4(),
                            })))
                            .collect();

        debug!("new todos: {:?}", new_todos);
        self.todos.set(new_todos)
    }

Calling both of those while using use_context breaks the app with:

panicked at 'context not found for type' and Uncaught RuntimeError: unreachable executed and I'm not sure why. Before I open a dedicated issue, do you have any ideas on what's going on here? Have been wracking my brain about it for a couple of days now 😅

@lukechu10
Copy link
Member Author

Yeah that does seem like a bug in Sycamore.
The reason for this is because Indexed and Keyed do not extend the reactive scope they were created in into the child scopes that are created during reconciliation. Contexts work by walking up the scope tree and therefore it can't find the parent scope where the context was created.

This also reminds me that the context not found error should definitely be improved. The location should point to the usage instead of sycamore internals.

Sorry for the trouble

@haywoood
Copy link
Contributor

No trouble at all! Thanks for taking a look, it's exciting to possibly find a bug and help improve things 😃

And I think I follow your explanation, though how come in my example repo the action that doesnt update a collection in the same component works?

haywoood added a commit to haywoood/sycamore that referenced this issue Nov 6, 2021
Does what it says on the tin.

Closes sycamore-rs#279
lukechu10 pushed a commit that referenced this issue Nov 6, 2021
Does what it says on the tin.

Closes #279
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-examples Area: examples good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants