Remove XHPRoot, seal `x:node`/composable-element to x:primitive and x:element

hhvm / xhp-lib

Class libraries for XHP. XHP is a Hack feature that augments the syntax of the language such that XML document fragments become valid Hack expressions.

http://facebook.github.io/xhp-lib/

MIT License

1.38k stars 160 forks source link

Remove XHPRoot, seal `x:node`/composable-element to x:primitive and x:element #248

Closed fredemmott closed 4 years ago

fredemmott commented 4 years ago

x:composable_element (aka x:node in #247) is extended by x:primitive and x:element
both of these implement XHPRoot
AIUI the XHP core framework needs to be aware of all subclasses, so there can't be more ones that are working
so, all XHPRoots are x:nodes and all x:nodes are XHPRoot
so the interface gains us nothing. Kill it?

fredemmott commented 4 years ago

Also, I don't think that :xhp needs to exist as a distinct class to node - it's only used as a holder for static functions, and as the base class for node.

ssandler commented 4 years ago

I like x:node and I think that it can replace XHPRoot. I don't think XHPRoot needs to exist.

:xhp doesn't really need to exist, but I do think it makes things like :xhp::disableChildValidation() read more clearly. These are settings that impact all XHP classes and the class name makes that pretty obvious. :x:node::disableChildValidation() would be a little more opaque.

fredemmott commented 4 years ago

Perhaps the static methods should move to be functions in the Facebook\XHP namespace?

fredemmott commented 4 years ago

or in those cases, in the Facebook\XHP\ChildValidation namespace we have for the trait-based approach

ssandler commented 4 years ago

Yeah that'd work just fine too!