-
Notifications
You must be signed in to change notification settings - Fork 375
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
[DOM Parts] Add new declarative syntax and update iterative proposal #1023
base: gh-pages
Are you sure you want to change the base?
Conversation
This removes some options that we have decided against. 1. DocumentPart and ChildNodePart cache child parts 2. DocumentPart and ChildNodePart clone 3. Partial attribute updating is supported with either array value or TemplateStringsArray
``` | ||
|
||
This could be exposed on the imperative API to be consumed in JavaScript by | ||
application logic. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to punt, but have we considered giving the metadata as a string rather than a string array? People may want to use a variety of metadata encodings, e.g. JSON, and it'd be easier to just pass along the raw contents and let them split by spaces if they'd like to do that.
Related, but we'll want to specify how to escape }}
inside of part syntax, and how to escape {{
inside parsepart
html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah it's not clear, but the reason metadata is a string is not because we split the string, it's because you might actually have metadata in two places for the same part.
{{# metadata1}}foo{{/ metadata2}}
I'll call this out.
Instead of `AttributePart` having a single string `value`, it could optionally | ||
take an `Array` that contains values that should be concatenated together. This | ||
allows updating individually parts of the attribute without needing to serialize | ||
the entire string. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of `AttributePart` having a single string `value`, it could optionally | |
take an `Array` that contains values that should be concatenated together. This | |
allows updating individually parts of the attribute without needing to serialize | |
the entire string. | |
Instead of `AttributePart` having a single string `value`, it could instead | |
take an `Array` with values that will be interleaved with those from the `strings` array. |
If we go with the strings
design, we might want to require that values
be an array, to be interleaved with the strings
. I suppose we could take a non-array value if there's only two strings, but in practice I expect template systems to just always pass through an array here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to think more about the DX of this.
I'd like to get a bit of clarification on syntax around parts with content and metadata. So, imagine a part that represents email content. It wants to provide both the binding expression metadata and the SSR'd value of the email. Would that look like this?... {{# email.expression.here}}[email protected]{{/}} It might be good to clarify this in the document. |
8b6f593
to
0bc28f3
Compare
If you agree, please downvote this proposal. By imposing extra curly brace in attributes you are:
From prospective of normal developer and manager What is available on the platform as of now?
Both syntax are available for DX as
And all of this usefulness you want to dump in favor of custom solution? |
@sashafirsov We're not committed to the |
@rictic , even if the syntax just a placeholder, the principle of attributes vs dom parts markup by same convention is important to separate on current discussion level. It can be done as in backward compatible as in breaking fashion. The parts within attribute already have a built-into-browser solution: XSLT convention. The dom parts as well: either as custom element or I see the proposal as attempt to swing away from ability to reuse the existing platform features along with complimentary advantages without matching compensation. The There is a compromise which can/should come with separate proposal: the source DOM alias: |
Please take the discussion of your proposal elsewhere. This issue is specifically about template / DOM part API. |
da053bb
to
81ead89
Compare
I added explicit mention of this in the declarative metadata section. How does that look? |
To respond to Sasha briefly. I think we are operating with the assumption that the majority of developers are working with frameworks, and I don't know of any popular frameworks that use XSLT syntax for templating. This has been true for many years. The choice of I think a number of us are also confident that we want a declarative syntax for template output, rather than just an imperative solution, which to me rules out template literal syntax in HTML. Template literals also have no concept of "start" "stop" ranges, and are merely insertion points for expressions, and we'd want our declarative syntax to be quite a bit more expressive. I think |
Another important point is that the syntax is also an instruction to the browser and may very well not be directly exposed to all users. In my world, Lit-style templates would still use JS template literals with standard JS syntax, but be able to very easily use the browser's syntax to create an HTML template with parts: const makeTemplate = (strings) => {
const template = document.createElement('template');
template.setAttribute('parseparts', '');
template.innerHTML = strings.join('{{}}');
return template; The import part here is that the delimiter ('{{}}') is valid in attribute position, which rules out elements and comments. |
I like the direction this is going. If I'm understanding correctly, the most important feedback should come from the server-side generating frameworks. If that is correct, I would encourage, if possible, surveying developers not just from the node-centric world, but include perhaps some representatives from beyond, a small slice of which may be using XSLT to generate the HTML. It seems that the key will be how easy it would be for early adopter users initially to extend the template syntax to support the desired output. Asp.net razor pages have support for custom tag helpers/ attribute helpers, which would allow for syntax that is defined in an elegant, semantic way (could even use xslt tags if so desired), which would emit the needed markers, similar to what it sounds like lit would support. XSLT supports user defined functions which could also generate the desired output, I believe. So it seems to me looking for syntax that matches the existing authoring / templating languages on the server side may not be as important as their ability to flexibly emit what is needed, but I might be missing something. I suspect it may not matter all that much if {{}} is used versus ${}, for example, echoing what @justinfagnani said. |
]); | ||
// Syntax to be improved. Here, a new AttributePart is created between each string. | ||
const part = AttributePart(document.getPartRoot(), element, "href"); | ||
part.value = ["mailto: ", email]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should think about making this behavior consistent and intuitive across the case where there are statics vs when there aren't. We also need to work with trusted types, so that we can set part.value
to a trusted type in a page where trusted types are enforced.
One proposal:
An attribute is said to be fully controlled by an AttributePart's value if the statics are ['', '']
. In that case, commit behaves like:
#commitFullyControlled() {
const value = Array.isArray(this.value) ? this.value[0] : this.value;
if (value == null || value === false) {
this.element.removeAttributeNS(this.namespaceURI, this.localName);
return;
}
this.element.setAttributeNS(this.namespaceURI, this.localName, value);
}
Note: if value
is an array, we use its first element. This is to be consistent with the behavior when there are nonempty statics. null
, undefined
, and false
when set on a fully controlled attribute remove it from the element.
If the attribute is not fully controlled by the value, then commit behaves like:
#commitWithStatics() {
const value = Array.isArray(this.value) ? this.value : [this.value];
let pieces = [this.statics[0]];
for (let i = 1; i < statics.length; i++) {
pieces.push(value[i - 1] ?? '', this.statics[i]);
}
return pieces.join('');
}
Note: if the values array is too short, then the ?? ''
will write it as empty string, and excess elements are ignored.
And for completeness:
commit() {
if (this.statics.length === 2 && this.statics[0] === '' && this.statics[1] === '') {
this.#commitFullyControlled();
} else {
this.#commitWithStatics();
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the wording a bit in this section to more cleanly spell out how statics and values fit together.
I'm not sure on statics = ['', ''] being the boundary condition. Why not just statics = undefined in the case where the AttributePart was created without statics? We can throw an Error during construction if someone tries to create an AttributePart with a static array that is less than 2 in length.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
['', '']
may be more elegant, but undefined
may be more performant, and I'd prefer the more performant option. No objection from me for undefined
81ead89
to
ce92803
Compare
## Choice of marker | ||
|
||
The `{{}}` and `{{#}}{{/}}` are reasonable DOM part markers, but this is open to | ||
proposals. It's possible to even allow the page to decide ewhat their markers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: s/ewhat/what
This PR makes various changes against the current DOM parts proposal:
{{}}
for a self-contained part,{{#}}{{/}}
for part with content. Parsing is enabled with aparseparts
attribute on any DOM node.DocumentPartRoot
orChildNodePart
are now containers for other nested parts. Browsers will need to maintain the nested list of parts.DocumentPartRoot
andChildNodePart
and returns a newPart
.