stoically
v0.9 API
Follow-up from https://github.com/stoically/syn-rsx/pull/23
Issue for general API discussion of the upcoming v0.9 release, which will have a rather large breaking change upcoming: Node will be converted to an enum, and the node tree leafs are Node enums as well. This will come with additional type safety, but makes the nodes a bit harder to access in terms of API. Feel free to voice concerns or ideas.
Should NodeValueExpr implement Deref? Is it a newtype or a smart pointer?
After reading up a bit on what the book says about newtypes and smart pointers (here, here and here), I'm leaning towards NodeValueExpr being a smart pointer. Reason being: While it doesn't hold extra metadata, its main purpose is granting access to the type it is encapsulating and adding extra capabilities via a multitude of traits. Furthermore consumers mainly want work with the encapsulated type, so having to do explicit method calls just to get the Expr instead of being able to leverage reference coercion seems like the wrong move.
My understanding regarding the newtype patterns is that the main use cases are additional abstraction over encapsulated types – up to the point that it should protect internal type state from undesired modifications –, making APIs type safe through wrappers or specifically to work around the orphan rule. None if which is what NodeValueExpr is primarily doing/providing.
Ok, fair enough. I'm still on the fence about it, but I guess newtype vs smart pointers is not really a black vs white issue, it's more nuanced than that. And I like the renaming. NodeValue, to me, seemed like a newtype, and it being an Expr was just an implementation detail. NodeValueExpr makes it seem like it's an Expr (and it is), just with extra goodies.
but I guess newtype vs smart pointers is not really a black vs white issue, it's more nuanced than that.
Yeah, I'd agree.
NodeValueExpr makes it seem like it's an Expr (and it is), just with extra goodies.
That was the intention, glad it worked out. Also considered Value, ValueExpr and just Expr.
Something I realized: The impl TryFrom<NodeValue> for String isn't really discoverable in the rust docs?! It's really only discoverable through the examples. I wonder what a good way to expose that bit of information would be? Just some hand-written explanation in NodeValues doc comment? :thinking:
Is it implemented?
Cuz
impl TryFrom<&NodeValueExpr> for String
this one shows up.
Yeah, TryFrom<&NodeValueExpr>, but still, it isn't discoverable in the rustdocs, or am I blind? :thinking:
Oooh, I'm blind, it's there https://docs.rs/syn-rsx/0.9.0-alpha.1/syn_rsx/struct.NodeValueExpr.html#impl-TryFrom%3C%26NodeValueExpr%3E-for-String, right. Then it's fine :D
I know the issue is closed, but just wanted to chime in on this and say thanks for the new API -- having just reimplemented our view macro using it and I can say it's much nicer to have the type safe enum variants than the constant .name_as_string().unwrap() and whatnot before. Great update!
