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
Namespaces/aliasing #64
Comments
I'd love a more generic aliasing system - something like 'use' semantics; there's been some brief mentions of namespace support, which could also solve this. |
Any downsides for the 'use' case? :) |
Hmmmm. The problem with I think the better solution is figuring out how to implement true namespace support into XHP. We could treat XHP's faux namespacing like real namespaces, so
I'm not entirely sure what the best solution here is. |
I understand the problem with the '.'-reference, this is indeed not an option. I thought fredemmott was referencing to the real PHP namespace aliases to be used as prefix? namespace Main {
use Other as o;
echo
<container> // Would replace as Container (or the xhp__ replacement)
<o:element /> // Would replace as \\Other\Element (or the xhp__ replacement)
</container>;
} |
You could only require a leading colon if you have imported or defined an element with the same identifier. |
It gets significantly harder to parse and translate to standard PHP, if we have to keep track of imported files and their parsed XHP (read: basically not an option). What I mean by they is, there's no way when looking at a file to know if there are imported classes that may have the same name as the one you're creating. |
We're going to implement standard namespace semantics, with ':' being equivalent to ''. Examples and exceptions at https://gist.github.com/fredemmott/1e65842fadb9e66e2359 As a short term step to help migration, if <foo:bar /> does not resolve, HHVM will look for <:foo:bar /> - if it exists, it will use it, but log E_DEPRECATED. Prefix aliasing will be done via 'use', just like any other namespaced class. |
Does this mean that XHP class names should look like PHP class names (in which there's no requirement, but the most common convention is camel case instead of dashes)? Or will names be the same as now (lowercase with dashes), and the recommendation will be to alias the namespace to something short and lowercased as well? At the same time, React already uses capitalized/camelCase component names, so maybe it's not that bad of a change. |
The built-in classes are keeping the current names, and this is likely to stay (eg x:composable-element, not x:ComposableElement). I expect this will lead to continued use of the current convention. We're not going to explicitly recommend aliasing, or discourage it. The main reason this got prioritized was:
Being able to alias prefixes is a nice side-effect :) |
Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed. |
no thanks FBBot. |
Since the bot brought it up...what is the status of this? Right now it's difficult to use XHP from namespaced classes, because hack doesn't look in the global namespace for HTML, resulting in errors like "Unbound name: NonGlobalNamespace:div." |
Still planned, but no ETA and not a current priority. Most likely to happen at a hackathon sometime. |
I'll see if we can put this up on bountysource. Will need changes to both XHP-Lib and HHVM. |
Removed all the namespaces in my project because of this issue. |
It's still something we'd like to happen, and I'll happily review pull requests (both for XHP-Lib and HHVM parts). Unfortunately it's something I'd have to do on my own time instead of for Facebook :( |
As a simpler (but less ideal) solution, can hh just be taught to know that XHP classes are allowed to fall back to the global namespace (ie updating hack server to match the existing runtime behavior)? |
We don't want to do that as that means that we can't do it correctly in the future without a major BC break. |
cc @sgolemon given https://github.com/phplang/xhp |
Funnily enough, I was just working on this problem as well for the PHP version of XHP. My thought was something along these lines:
The idea being that we do reuse the same basic semantics as regular PHP namespaces, but they occupy a separate space. e.g. foo\bar is a separate namespace than foo:bar) In terms of implementation, what I have in my proof-of-concept patch is that a use statement adds a lookup to a per-file map (since PHP namespaces are file-scoped), and any XHP start tag checks the leading segment of its name against the map, to expand as it parses. What this approach doesn't accommodate is tags which don't want to be expanded:
I'm not sure that necessarily needs to be worried about, it smells like terrible code to me, and it can be dealt with by allowing root namespace modifying (e.g. |
So would this not be allowed? namespace Foo\Bar {
class :baz:boo {}
} |
What would this output? |
@jesseschalken I would say "Allowed, but not a good idea." Because now you have a tag sitting inside an effectively incompatible PHP namespace. How would you access that from a foreign file? For that, we'd need to fully integrate the two namespace types, and there's enough differences between them that it feels like shoving a square peg into a round hole. If someone has a clean idea for how that'd look, I'm game, but I don't like any of the obvious routes. @fredemmott |
Is this equivalent to not actually namespacing them, but:
|
@fredemmott Yes, in the sense that XHP's current namespaces aren't really namespaces. They're just labels that happen to allow colons and hyphens in them. This adds use semantics while leaving them still not-really-namespaces. |
Yeah. I don't see what's wrong with that?
Before PHP introduced namespaces people were already using I think it would be most straightforward and less surprising to PHP devs if XHP took the same route, enabled compatibility with PHP namespaces, and left You would end up with: namespace Foo {
use :ul, :li, :fb:thing;
class :baz:qux {}
echo <ul>
<li>Hello</li>
<li>World</li>
<li>
<Bar\boo />
<baz:qux />
<fb:thing />
</li>
</ul>;
}
namespace Foo\Bar {
class :boo {}
}
namespace {
use Foo\Bar\:boo as :bob:boo;
echo <div>
<Foo\baz:qux>
<bob:boo />
</Foo\baz:qux>
</div>;
} With the real class names as: namespace Foo {
use xhp_ul, xhp_li, xhp_fb__thing;
class xhp_baz__qux {}
echo <xhp_ul>
<xhp_li>Hello</xhp_li>
<xhp_li>World</xhp_li>
<xhp_li>
<Bar\xhp_boo />
<xhp_baz__qux />
<xhp_fb__thing />
</xhp_li>
</xhp_ul>;
}
namespace Foo\Bar {
class xhp_boo {}
}
namespace {
use Foo\Bar\xhp_boo as xhp_bob__boo;
echo <xhp_div>
<Foo\xhp_baz__qux>
<xhp_bob__boo />
</Foo\xhp_baz__qux >
</xhp_div>;
} This looks fine IMO. |
I disagree that I think use semantics ("prefix aliasing + implicit root lookup") provide everything I'd want in XHP namespacing, and it solves the main reason this issue got prioritized in the first place ("Hack requires globally unique class names" -> I can name my components |
Having a completely different namespacing system running in parallel to PHP's existing namespaces just to avoid putting a backslash inside a tag name definitely feels wronger. It's the same with React in JavaScript where, apart from modules,
Yes you can. This code works fine. namespace Foo {
class Bar_Baz {}
} I don't see why you'd do it, but it works fine.
With interop with PHP namespaces you can do namespace Vendor\Product\ui;
class :thing {} namespace Blah;
use Vendor\Product\ui;
echo <ui\thing />; or even namespace Blah;
use Vendor\Product\ui\:thing;
echo <thing />; Just build in your existing namespace hierarchy.
I write React components in the same file as standard JavaScript classes all the time. In fact, I don't even have a JavaScript file that contains a React component without a class, function, constant, variable or type definition in the same file. Why should XHP stop me from doing the same? Projects already have their own namespaces like |
Okay, thinking of the colon as having no significance... Yes, I agree with you. And yes, there is certainly precedent when compared to the underscore's role pre PHP-5.3. I still don't like the look of a backslash in an XHP tag, but I could get over that. (I never liked the backslash as a namespace separator to begin with) For that route we'd just need backslashes respected in tagnames (along with some mangling updates) In terms of my goals (which I realized I only put in the FB post, but not here), I was hoping to make XHP tags more composer friendly, and this does accomplish that. A library package might look like:
Could be autoloaded with the usual
Functionally speaking it won't be hard to get this working, and since it'd use PHP's existing name resolution mechanisms there would be less to get wrong. I like getting more benefit for less work, so that might be enough for me to swallow something as ugly as @fredemmott is anyone on your end interested in weighing in? Happy to drop by and freeload some lunch to talk it out if someone with an opinion is available this week (I'm out of the country next week and into the following). |
I'm tight on time this week and can't really look in detail (especially comparing advantages/disadvantages to the previous conclusion). I can leave notes here next week for when you're back and talk after? Also, @Swahvay: ping :) |
@sgolemon, I agree with basically the entirety of your last comment. The syntax looks hideous, but I always thought the \ namespace separator was hideous to begin with too. I keep thinking of the old adage, "code is written to be read," and although it's not pretty
is pretty clear what's going on. I sort of wish we could go the route of treating colons in XHP like a namespace separator, so As I see it, the only remaining confusion from adding namespaces to XHP with the standard separator will be, "What are colons for in XHP classes?" |
That reminded me... this I really dislike:
Just another valid token for class name (sort-of), make it deprecated though? |
I guess you mean PSR-n? They're supported via the classmap approach, and the Hack xhp-lib itself uses that - the standalone autoload.php is just for people who choose not to use composer. |
That would really be unfortunate, since the X in XHP is for XML, which uses the colons. Plus for people (like myself) who don't use PHP namespaces in their projects, I find the faux namespaces of XHP very nice. Also agree that |
The main issue there, in my mind, is class declarations:
Maybe we expand classes+namespaces to allow syntax like this as well, but that's a separate dimension to the problem.
Classmap is a bit on the klunky side, actually. But no, that's not so much the problem; I've added a class loader: https://github.com/phplang/xhp-lib/blob/master/src/ClassLoader.php which is basically PSR-4, but using colons instead of backslashes. The loader gets hooked in an
Not quite first-class, but it works. That said, I'd prefer to have a meaningful way to make XHP fit into PHP namespaces, hence involvement in this thread.
I think I just threw up in my mouth a little. |
Where are we at? as far as I can tell:
I still think that https://gist.github.com/fredemmott/1e65842fadb9e66e2359 is the best approach; in addition to the reasons in that gist, as xhp classes must either be in the root namespace or only have a single leading colon, it also solves the PSR-4 friendliness issue. |
I'm hoping to get to this around July, maybe a bit later. This is getting more and more painful. The main delay is that currently, this involves changing:
In a few months, we should be at a point where only the FFP needs changing. |
Work-in-progress pull request at facebook/hhvm#8578 - this led to us finding issues with it. Current status and open questions at https://gist.github.com/fredemmott/7316287e3fb9cc43b312b3e51c155ba5 |
As of tomorrow's nightly builds, I believe this is usable, but with codemods/migrations required, and non-default hhconfig and hhvm options. In particular, with these options enabled:
#216 contains xhp-lib tests of this, though needs extending to cover the runtime The majority of issues labelled 'v4' must be completed before we can flip the defaults and ship a new xhp-lib. codemods/migrations are being added to HHAST to do the parts that can be done in isolation, e.g. hhvm/hhast#267 and hhvm/hhast#268 |
|
Think we're done, except for flipping defaults and banning 'children' declarations. Doing a final internal check for feedback.
|
Also, except for the final one, that's unlikely to be the new API - there will be more manual work involved e.g. instead of simply replacing - with _; that's the minimum to check functionality. |
Implementing a alias to the current prefix '.' would be very efficient. I'm not sure how clean this would look. Alternative could be leaving out the prefix works like an alias to the parents prefix, but this would require to prefix the current default elements (when the parent is not).
or
The text was updated successfully, but these errors were encountered: