-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
feat: new private node property "_withoutClosingTag" #48
Conversation
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.
The implementation should be done here because this block is responsible for the type of render
Lines 192 to 206 in 903037c
if (isSingleTag(tag)) { | |
switch (closingSingleTag) { | |
case 'tag': | |
result += '></' + tag + '>'; | |
break; | |
case 'slash': | |
result += ' />'; | |
break; | |
default: | |
result += '>'; | |
} | |
result += html(node.content); |
I think that it is necessary to introduce another type, for example, byProperty
.
- You are adding an element that must be single
options.singleTags = ['html']
- Refactor
isSingleTag
const asTag = '></' + tag + '>';
const asSlash = ' />';
const asDefault = '>';
switch (closingSingleTag) {
case 'tag':
result += asTag;
break;
case 'slash':
result += asSlash;
break;
case 'byProperty':
result += node._closingElement ? asTag : asDefault;
break;
default:
result += asDefault;
}
Don't forget to add this to the documentation.
thanks
@@ -204,6 +204,8 @@ function render(tree, options) { | |||
} | |||
|
|||
result += html(node.content); | |||
} else if (node._withoutClosingTag) { |
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.
_withoutClosingTag
The name is not very suitable because tag means that there is an opening and closing element.
It seems more appropriate to me that the _closingElement
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.
Regarding semantics, element
in HTML refers to the whole thing - an element has start/opening and end/closing tags.
I agree double negation through the use of without
in the name is unnecessary, but if a name such as _closingTag
creates confusion, maybe we could use something like _needsClosing
or _shouldClose
or something similar?
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.
Regarding semantics,
element
in HTML refers to the whole thing - an element has start/opening and end/closing tags.
It is considered that if the element is single such as <hr>
or <img>
then it is the element
if the element has a closing element then the tag
is used
@Scrum I understand this could be a solution to posthtml/posthtml-mso#1 ? |
Yes, you can bypass all child elements by the found match and set them a parameter |
Actually, WHATWG HTML spec uses the word
By default |
The fact is that we go beyond recommendations by manipulating the After a little reflection, I realized that we can do even better if we pass the type used for result += typeof node._closingElement === string ? as[node._closingElement] : node._closingElement ? asTag : asDefault; |
Notable Changes
The PR adds support for a private property
_withoutClosingTag
:will result in
Commit Message Summary (CHANGELOG)
Type
SemVer
Issues
The PR is primarily used to implement posthtml/htmlnano#29, to omit those optional closing tags (without producing invalid HTML). It might also help to handle the case I mentioned in posthtml/posthtml-parser#9 (comment).
So far
_withoutClosingTag
should be considered as a private property ofnode
(with_
prefixed) and shouldn't be documented, but we could make it public in the future.Checklist