Replace `name_as_*` and `value_as_*` methods on `Node` with `TryInto` impl

[stoically]

Keep the methods on the Node but mark them deprecated.

[stoically]

This actually doesn't work since Node::name/value are options. I still feel like the API could use some improvement, just not sure how. Open to suggestions.

[Zizico2]

My suggestion here would be to do handle it like syn does. Have an Enum with the possible variants and a correspondent struct for every one.

[Zizico2]

This may be my least favourite thing about syn-rsx. The API doesn't feel very rusty. I just end up unwraping a lot. And then back tracking and discovering where the unwraps can and cannot fail.

[Zizico2]

This actually doesn't work since Node::name/value are options. I still feel like the API could use some improvement, just not sure how. Open to suggestions.

It would still work, no? We can always .map the options. Would not be as clean as a single try_into, but it would be more idiomatic than the current solution. I still prefer my suggestion above though.

[stoically]

The API doesn't feel very rusty.

I'd agree.

My suggestion here would be to do handle it like syn does. Have an Enum with the possible variants and a correspondent struct for every one.

If I understand that right that's a suggestion regarding the node children, right? So, basically something like

enum Node {
  Element(NodeElement),
  Attribute(NodeAttribute),
  ...
}

I was considering this when implementing. The reason I decided against it was: I wanted to be as close to the browser DOM as possible and all nodes basically share the same fields, so I thought I would end up duplicating the same node for every type of node, with only slight adjustments. Which also felt a bit clumsy from the consumer side of things, as you have to handle many types suddenly, instead of only Node. And since the type of the node is available via Node::node_type currently, it felt like that's enough of a context information to work with the node when traversing. I'm however open to rethink that decision and would be interested in suggestions that don't require duplicating things too much.

~~Also not exactly sure how it'd help in terms of TryInto<T> implementations, as those would still need to be implemented for the node value and type specifically, unless I'm missing something?~~ Turns out it would make implementing TryInto<T> much cleaner.

My suggestion here would be to do handle it like syn does. Have an Enum with the possible variants and a correspondent struct for every one.

Right, I could implement TryInto on NodeName either way, good point. For Node::value I'd have to do https://github.com/stoically/syn-rsx/issues/15 first.

[stoically]

I just end up unwraping a lot. And then back tracking and discovering where the unwraps can and cannot fail.

That's a strong argument to live with less DRY code tho, since needing to depend on manually understanding the docs to do safe unwraping feels almost not type safe.

[stoically]

Started to look into an enum style Node implementation, and it actually isn't that bad in terms of duplication. Seems much cleaner.

[Zizico2]

Let me know if I can help. I agree with everything you said. And about the duplication part, I'm not that familiar with syn-rsx's internals, but we can implement the common stuff on the enum directly, no? And maybe have the different structs implement a common trait.