I think I would prefer to have an explicit newtype for this (e.g. NodeId) that wraps u64 and provides From conversion methods.

That would make the code easier to read (effectively adding more documentation) and would make it easier to change the internal representation later if needed.

I think I would prefer to have an explicit newtype for this (e.g. NodeId) that wraps u64 and provides From conversion methods.

Hmm... so something like:

struct NodeId(u64)

I can see advantages of this (for one I could implement conversion function to and from slotmap keys, which would reduce a bunch of boilerplate). Would you want that type exposed externally in the LayoutTree trait instead of u64, or would it just be for internal use?

I think I would prefer to have an explicit newtype for this (e.g. NodeId) that wraps u64 and provides From conversion methods.

Hmm... so something like:

struct NodeId(u64)

Yup, that's exactly what I had in mind :)

I can see advantages of this (for one I could implement conversion function to and from slotmap keys, which would reduce a bunch of boilerplate). Would you want that type exposed externally in the LayoutTree trait instead of u64, or would it just be for internal use?

I think it would be reasonable to export it as well, but I don't have strong opinions on this

Ok, so I've introduced a NodeId newtype as suggested. And I've gone ahead and replaced all public uses of type taffy::node::Node = slotmap::DefaultKey with it (and then deleted taffy::node::Node). This is super nice as it makes slotmap an implementation detail that is no longer part of the public API at all.

NodeId has From/Into conversions defined for u64, usize (which are intended for external interop), and slotmap::DefaultKey (which we use internally).

Do you want to add this to the release notes in this PR or in a future one?

Do you want to add this to the release notes in this PR or in a future one?

Release notes added :)

Performance seems to be roughly neutral :) Merging!