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.

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

Allow parsing of HTML-like Comments #7802

Closed
xtuc opened this issue Apr 25, 2018 · 6 comments
Closed

Allow parsing of HTML-like Comments #7802

xtuc opened this issue Apr 25, 2018 · 6 comments
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser

Comments

@xtuc
Copy link
Member

xtuc commented Apr 25, 2018

Choose one: is this a bug report or feature request?
Bug I guess

Input Code

<!-- test --->

Expected Behavior

undefined

Current Behavior

{ SyntaxError: res/ex-html-comment.js: Unexpected token (1:0)

> 1 | <!-- console.log("foo") -->
    | ^
  2 | 
    at Parser.raise (/home/sven/Documents/xtuc/talks/understanding-the-differences-is-accepting/node_modules/babylon/lib/index.js:779:15)
    at Parser.unexpected (/home/sven/Documents/xtuc/talks/understanding-the-differences-is-accepting/node_modules/babylon/lib/index.js:2082:16)
    at Parser.parseExprAtom (/home/sven/Documents/xtuc/talks/understanding-the-differences-is-accepting/node_modules/babylon/lib/index.js:3158:20)
    at Parser.parseExprSubscripts (/home/sven/Documents/xtuc/talks/understanding-the-differences-is-accepting/node_modules/babylon/lib/index.js:2758:21)
    at Parser.parseMaybeUnary (/home/sven/Documents/xtuc/talks/understanding-the-differences-is-accepting/node_modules/babylon/lib/index.js:2737:21)
    at Parser.parseExprOps (/home/sven/Documents/xtuc/talks/understanding-the-differences-is-accepting/node_modules/babylon/lib/index.js:2646:21)
    at Parser.parseMaybeConditional (/home/sven/Documents/xtuc/talks/understanding-the-differences-is-accepting/node_modules/babylon/lib/index.js:2618:21)
    at Parser.parseMaybeAssign (/home/sven/Documents/xtuc/talks/understanding-the-differences-is-accepting/node_modules/babylon/lib/index.js:2565:21)
    at Parser.parseExpression (/home/sven/Documents/xtuc/talks/understanding-the-differences-is-accepting/node_modules/babylon/lib/index.js:2518:21)
    at Parser.parseStatementContent (/home/sven/Documents/xtuc/talks/understanding-the-differences-is-accepting/node_modules/babylon/lib/index.js:4077:21)
  pos: 0,
  loc: Position { line: 1, column: 0 },
  code: 'BABEL_PARSE_ERROR' }

Possible Solution

Ignore it or update Babylon (it could be a good first issue?).

Context

Maybe some peoples are still using it?

Your Environment

software version(s)
Babel 7
Babylon
node v6.11.4
npm
Operating System

See spec: B.1.3 HTML-like Comments

Comment::
  SingleLineHTMLOpenComment
  ...

SingleLineHTMLOpenComment::
  <!--SingleLineCommentChars
  ...
@xtuc xtuc changed the title HTML-like Comments Allow parsing of HTML-like Comments Apr 25, 2018
@KFlash
Copy link

KFlash commented Apr 25, 2018

You did not mention if this was in sloppy mode or module code, but note that <!--` in module code is not considered a comment start , but allowed in sloppy mode.
However. This is an common issue among the majority of other JS parsers. E.g. Esprima got this right in sloppy mode, but doesn't throw in module code. Shift got it right both in sloppy and module code.

@loganfsmyth
Copy link
Member

loganfsmyth commented Apr 25, 2018

I'm not strictly this, but I don't know that I'd call it a spec violation because it's part of Annex B, which is optional, and Babel has generally not handled that stuff. For instance, we definitely don't handle all of the old non-standard block-level function hoisting defined in annex B. I did file #5266 last year to track the possibility though.

@hzoo
Copy link
Member

hzoo commented Apr 25, 2018

Personally, I'd I don't think we support support this or anything in Annex B - we never have and there's already enough issues that are way higher priority. We don't have to (not a browser) so we actually have the option not to (this is true for anything really). Basically I think we need to think about priority (code that is normally written/used), and there's plenty of that such that I don't see this happening unless this is a really good use case - if it's just a curiosity/completionist kind of view then I would ask that we not try to pursue this kind of thing?

@KFlash
Copy link

KFlash commented Apr 26, 2018

I noticed this has never been supported in acorn either. Off topic. I Looked at the lexer and noticed it possible to gain 3% perf boost. Move WS skipping to top of the switch and all other rare WS only validate them if hit max ascii chars - 128 and above. And only break the loop in the default clause. No ws skipping.
It's also possible to gain perf if rewrite fromCidePoint. It's too bloated.

@xtuc
Copy link
Member Author

xtuc commented Apr 27, 2018

Ok, I haven't realized that Babel doesn't support the Annex B (I don't think it's documented somewhere neither).

First, I don't particularly care about allows HTML comments in Babel, it seems like a very uncommon use-case. Unlike the non-standard block-level function hoisting we can just parse it (and emit it again if needed).

On the other hand, I think that it could be a good first issue for someone wanting to contribute to our parser/babel.

@danez
Copy link
Member

danez commented May 15, 2019

We do support html comments in script mode. Till recently we did not allow them on the first line though: #9760

@danez danez closed this as completed May 15, 2019
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Aug 14, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser
Projects
None yet
Development

No branches or pull requests

5 participants