-
Notifications
You must be signed in to change notification settings - Fork 37
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 ybc to yew 0.19 #25
Conversation
Nice! I'm stoked to see this. Just let me know when you want me to give it an eye, and then I'll give it a review. |
So far I've verified that the simpler components definitely work with the new code (the simple wrapper components which I have managed to replace with function components for the most part). Now the problem is that this is the extent of the advanced ybc components I actually use in my app at the moment - I'm considering adapting some of the more complex ones such as File as well, however my project-internal wrappers around file input and similar things all do some special magic, for example accepting drag-drop events in addition to normal use, so I'm not entirely sure how to proceed (if I should move these features into ybc as well or if I can find a way to wrap ybc while also providing additional features on top - I'll see). I'm considering adding a demo page as a big example to ybc - basically a page mirroring parts of the docs on bulma.io, just implemented with yew and ybc, so we'd have both a nice demo page to show off the framework, as well as a sort of "testing ground" to look out for obvious breakages. Still thinking about whether I can afford the time to actually do that though. I'm now gonna check of some off the question in my OP that I've already cleared up. |
@thedodd so while my code probably is not ready for review yet, I have some higher-level questions that you could maybe answer or give your thoughts about right now (or just tell me that I'm now the maintainer and get to decide myself lol): On the first unchecked item on my list: I think I've mostly figured this out and it's fairly simple to do by making use of wasm-bindgen, but this does add one more direct dependency in wasm-bindgen. I could switch to the yew-provided version of it, On the router side, I'm wondering about two things: Whether we want to continue to directly support router at all (I think it would still be nice), and if so, how to name our components lol. |
Hi @thedodd, could you merge this? I have not looked at the code of this change too much but I am using ybc with this change with yew 0.19.3 and it seems to be working quite nice from a library user perspective. I also can see that there haven't been any update to this repo for 6 months now, is this still an active project? |
I'm very sorry for my lack of progress on this. Without this demo page, I don't really have an option for testing whether all components work - specifically the more complex ones, like the Router integration stuff and the File component. |
My 2c: Very interested in seeing this merged. As a first time user of yew it seems that half of the eco system is not on 0.19 and 0.18 seems to be vastly different. As far as I can tell, this library used to be the de-facto standard for components library for yew, but it not being on 0.19 putting it at danger of obsolescence. |
1334ce3
to
e515cc4
Compare
As I said in my last comment, I think this is ready to get merged, but I guess @thedodd or someone else would want to review the code first. The last change I pushed raises the MSRV to 1.62 btw.
Yeah - I think the catch here is that 0.19 reduced the boilerplate for defining components a lot. At the same time, Yew 0.19 didn't add a way to "pass down the rest of the props to a child node". This is being tracked here yewstack/yew#1533 but nothing is really happening there. In reality the combination of these circumstances means that component libraries are now much less useful than they could be - if the upstream library forgot to expose an event handler you need, you have to either add a wrapper and hope that it bubbles up, or open an issue/PR on the upstream. With how easy it is to define your own component, I suspect this effort is often just not worth it, and people end up just defining their own component which exposes the functionality they need. I know I do. |
As to #27 - this is something that's not guaranteed for this branch either. Once this PR is merged, it would likely be a good idea to make this repo's master branch target the yew master branch. |
During development of my demo page I noticed that using The only downside of this change is that internally we now have to do explicit clones, but from an API and performance perspective it is superior in every way. |
Yew v0.20 is out. Seams like the logic for modals will need an update to use the new Context API. @Follpvosten do you plan to work on this? If not I might. |
@Follpvosten I don't have time right now to do a full review, but what I think we could do is cut a beta release! That will give us a chance to merge, and folks can use and experiment if they are so inclined. @SionoiS yea, for the 0.20 ... I could merge this PR, and then you could open a new PR with the 0.20 updates. What do you think? We could just make that the next alpha. |
@SionoiS right - they deprecated non-worker agents, so you will have to build an API on top of ContextProvider and channels instead. @thedodd merging and cutting a beta release does sound reasonable. After that I could open a separate PR where I add my demo page, so other people (or me when I can find the time) can complete it at some point. |
Merge this as beta then v0.20 in separate PR, sounds good to me! |
I'm working on updating the whole codebase to support yew 0.19, while also making the majority of components function components (because simple wrappers like ybc does are a really good usecase for function components). I know that this is close to a complete rewrite, but in my opinion, it's for the best of this crate. I'm open to suggestions for changes.
Currently this is a draft because
File
,Input
,Radio
,Select
andTextArea
.yew_router::components::Link
component now. TheRouterButton
component doesn't exist in yew_router anymore, so we might want to a) deprecate the ybcRouterButton
or b) implement it differently by taking inspiration fromLink
; however we could also consider removing the direct yew-router support completely. Affected is everything gated by the router feature, but most importantly RouterButton.I've replaced almost all places where aLooks like this is not gonna be an issue! I might still look into making some macro invocations even shorter though.Classes
instance was created with aclasses!
macro invocation. I don't know why it wasn't done like that before and would be happy to change it back if there's a good reason. (I've only avoided it where the logic was complex and I was too tired to think about it reasonably.)Aggressively converting most components to function components might also not be wanted by the maintainer(s) here, so I'm open to changing some of them back. I've only avoided it for components that store state or connect to agents for now; we could go all the way and make those fn components as well, or we could roll back, I don't really care. However I do think that the vast majority of components in ybc benefits hugely from being turned into a function component because most of them really are just simple wrappers.I don't think there's any objections to this.