Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add support for HTML comments #84

Merged
merged 6 commits into from
Jul 25, 2019
Merged

Add support for HTML comments #84

merged 6 commits into from
Jul 25, 2019

Conversation

jviide
Copy link
Collaborator

@jviide jviide commented Apr 19, 2019 •
edited
Loading

This pull request adds support for HTML5-style comments (with relevant tests). Everything between comment delimiters <!-- and --> gets ignored during parsing. For example the the following template gets parsed as h("div", null):

html`
  <div>
    <!-- Everything between the comment
         delimiters get ignored, including <tags>,
         newlines and ${'variables'} -->
  </div>
`;

A deviation from the HTML5 standard is that the text between the delimiters can start with > and ->.

html`
  <div>
    <!-->This is also ignored<!-->
  </div>
`;

The .mjs.br size grows from 540 to 564 bytes for the regular build. The mini version grows from 376 to 398 bytes.

Closes #79.

@jviide jviide mentioned this pull request Apr 19, 2019
Closed
@jviide jviide requested a review from developit April 29, 2019 10:01
marvinhagemeister
marvinhagemeister approved these changes Apr 29, 2019
View reviewed changes
Copy link
Collaborator

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎉

@dy
Copy link

dy commented Jul 9, 2019 •
edited
Loading

@jviide can we possibly adjust this PR a bit to support other types of unclosed tags, ideally customizable by consumer?

Standard declarations:
<?xml version="1.0" encoding="UTF-8" ?>
<!doctype html>
<![CDATA[ ...  ]]>
<!-- comment -->
<!--[if expression]> HTML <![endif]-->

Standard unclosed tags:
<br>
<hr>
...

Custom tags:
<...>
<#selector>
<.selector>

(related issue #91 (comment))

That will be super-useful for consuming frameworks and should not add much size to the parser.
Eg.

import htm from 'htm'

// the dict of reserved single tags with (possibly) handlers?
htm.single = { '!--': true, '?xml': true, br: true, hr: true}
// the default is { '!--': true }

@developit
Copy link
Owner

developit commented Jul 9, 2019 •
edited
Loading

@dy that would add quite a bit of size since currently the parser tags/tokens get inlined.

In general, I don't want to do anything that takes htm off of its very narrowly scoped path of being a JSX equivalent. That being said, if you think it'd be worth having a project that exposes @jviide's parser as a standalone module, I could see that being interesting and worthwhile. It just can't really be part of htm itself.

@jviide
Copy link
Collaborator Author

jviide commented Jul 9, 2019

Merged in the changes from the current master. The final size is 588 B (up from 563 B) for the regular build and 417 B (up from 394 B) for the mini build.

@dy

This comment has been minimized.

Sign in to view
@jviide jviide merged commit 526bb7b into developit:master Jul 25, 2019
@jviide jviide deleted the comment-support branch July 25, 2019 21:52
luwes added a commit to luwes/sinuous that referenced this pull request Jul 31, 2019
@luwes
Add comments support in HTM
5651843 
developit/htm#84
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@developit developit developit approved these changes

@marvinhagemeister marvinhagemeister marvinhagemeister approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Silently fails with HTML comments
4 participants
@jviide @dy @developit @marvinhagemeister