Skip to content
This repository has been archived by the owner on Apr 1, 2024. It is now read-only.

[Request] Custom special attributes #303

Open
Kilenaitor opened this issue Dec 3, 2021 · 4 comments
Open

[Request] Custom special attributes #303

Kilenaitor opened this issue Dec 3, 2021 · 4 comments

Comments

@Kilenaitor
Copy link

Background

For some context that inspired this request, currently working on a site using XHP for server-side HTML rendering. Looked into progressive enhancement frameworks for JS and found one I really liked called Unpoly. The way it works is by annotating the HTML with custom special attributes prefixed with up- (e.g. up-target, up-expand, etc.).

Motivation

XHP does support custom attributes already (data- and aria-) but there is no way to extend this list if there's a desire for more.

The only existing way I can think of doing this is by basically reimplementing all of the core XHP HTML elements by extending some new like "base up element" that enumerates all the attributes and then just smearing the HTML element attributes and extend via composition.

If there's some other way to do this, that'd be great. But otherwise, would it be possible to add this?

Proposal

In my mind, this would be a new HHVM INI config option that can be used to add additional special attribute prefixes. Something like hhvm.xhp.special_attribute_prefixes which is configured with a default of keyset['data-', 'aria-']

I'd then just be able to modify my INI file with:

hhvm.xhp.special_attribute_prefixes[] = 'up-'

and I don't have to do any additional code. I can just use the attributes right away.

@fredemmott
Copy link
Contributor

The runtime should actually already be fine with this - it needs xhp library and typechecker extensions though.

From an open source perspective, this would probably need to be a new language feature rather than a .hhconfig option - we definitely don't want to encourage libraries that require .hhconfig changes (or any other project-wide or global setting), but it's workable in a monorepo

@Kilenaitor
Copy link
Author

Kilenaitor commented Dec 5, 2021

it needs xhp library and typechecker extensions though

Assuming this change would need to be in the HHVM repo, then? e.g. here?

If you have some pointers, I'd be happy to try and contribute to this if I have time. I doubt it's urgent since I'm guessing no one has asked for this before me haha.

Main thing that's still unclear to me is how to actually configure which special attribute prefixes to allow. I'm assuming the "would probably need to be a new language feature" means rather than a config option it'd be some new special syntax for how to declare these (e.g. in addition to attribute something like special-attributes but with an actually-good name). Would still be nice though to have some way to extend/augment the existing XHP HTML elements with these special attributes. Unless the intention is to require reimplementing core elements whenever non-standard attributes are desired. While I'd understand that principle... it'd be a bit annoying haha.

But yeah. As said above, if you have some idea of how this would get done, I'd be down to give it a shot.

@lexidor
Copy link
Contributor

lexidor commented Dec 20, 2021

I am very interested in how this would be implemented. I am worried this will make xhp attributes more unsound. data- and aria- are unsound. If the information about which prefixes are special depends on the (base)type, you are left reimplementing the html tags anyway. If this information is dynamically configurable, this would make any attribute string prefix-thing possibly unsound.

Would it be okay if the renderer could be configured to special case prefixes like data-up- to render as up-? So in XHP, you'd write.

<p data-up-expand="JS" />

which would render as <p up-expand="JS"></p>?

protected final function renderBaseAttrs(): string {
$buf = '<'.$this->tagName;
foreach ($this->getAttributes() as $key => $val) {
if ($val !== null && $val !== false) {
if ($val === true) {
$buf .= ' '.\htmlspecialchars($key);
} else {
if ($val is \Facebook\XHP\UnsafeAttributeValue_DEPRECATED) {
$val_str = $val->toHTMLString();
} else {
$val_str = \htmlspecialchars((string)$val, \ENT_COMPAT);
}
$buf .= ' '.\htmlspecialchars($key).'="'.$val_str.'"';
}
}
}
return $buf;
}

Adding an if () {} here would keep type safety equal, at the cost of having to type out data- prefixes.

@fredemmott
Copy link
Contributor

I think we'd be very unlikely to provide a way to retrofit this to existing HTML classes:

  • it is essentially what the data- attributes are meant for
  • it would make migrating XHP attributes to be class properties much more difficult - we would already have to special-case data- and aria-, but these can be detected just at the callsite. Allowing plugin-extensions here would require looking at parts of the current tree that are not ancestors in the class hierarchy

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants