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 Sycamore to v0.6.0 #10

Merged
merged 4 commits into from Sep 14, 2021
Merged

Conversation

lukechu10
Copy link
Contributor

Updates sycamore to 0.6.0, removes patch dependencies

@lukechu10 lukechu10 marked this pull request as ready for review September 12, 2021 19:53
@lukechu10
Copy link
Contributor Author

For some reason, clicking on links causes a context error.

@lukechu10 lukechu10 marked this pull request as draft September 12, 2021 19:54
@lukechu10
Copy link
Contributor Author

lukechu10 commented Sep 12, 2021

Found it. This line here is not being called inside a reactive scope so it can't access the context:
https://github.com/arctic-hen7/perseus/blob/b7ad607861340c56bbfd504d6d2880108dbb0116/packages/perseus/src/router.rs#L145
That's because it's called when the anchor is clicked which happens during the global scope.

Copy link
Member

@arctic-hen7 arctic-hen7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really great, thanks so much!

@arctic-hen7
Copy link
Member

Okay that makes a lot of sense for the context error. Given though that match_route can't take any arguments, how would that be fixed? I'm only providing the routes in context because of the restrictions of the Route trait...

@arctic-hen7
Copy link
Member

Would you mind quickly running cargo fmt so tests pass?

@lukechu10
Copy link
Contributor Author

Would you mind quickly running cargo fmt so tests pass?

Done

@lukechu10
Copy link
Contributor Author

I'm only providing the routes in context because of the restrictions of the Route trait

What kind of restrictions are you thinking about? Object-safety? The way I would do it would be to use a global variable since there is only 1 router per app (albeit not being very idiomatic in Rust).

Alternatively, I could change something in Sycamore itself to prevent the router from handling click events. This way, we could attach our own click handler and manually call navigate. This is probably cleaner and we'll probably a config like this in Sycamore anyways.

Copy link
Member

@arctic-hen7 arctic-hen7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good!

@arctic-hen7
Copy link
Member

I'll fix the broken links issue after this is merged as part of the fixes to #8, so this should be good to merge if you're done with it.

@lukechu10 lukechu10 marked this pull request as ready for review September 14, 2021 02:16
@lukechu10
Copy link
Contributor Author

Alright. I'm done then.

@arctic-hen7 arctic-hen7 merged commit d71e633 into framesurge:main Sep 14, 2021
@arctic-hen7
Copy link
Member

Thanks so much, this is really helpful!

@lukechu10 lukechu10 deleted the sycamore-0.6 branch September 14, 2021 15:25
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.

None yet

2 participants