glimmer-vm icon indicating copy to clipboard operation
glimmer-vm copied to clipboard

Preprocessing into AST doesn't work with <!DOCTYPE> tags

Open timlindvall opened this issue 6 years ago • 5 comments

Hello!

Running a template with doctype tags (such as <!DOCTYPE html>) appears to cause preprocessing the template into an AST to break.

Example:

<!DOCTYPE html>
<html lang="en">
  <head>
    <title>Hi! I'm an Ember app!</title>
    <meta name="description" content="">
    <meta name="viewport" content="width=device-width, initial-scale=1">
    <link rel="stylesheet" id="default-css" href="/assets/my-app.css">
    {{content-for 'head'}}
    {{content-for 'head-footer'}}
  </head>
  <body>
    {{content-for 'body'}}
    {{content-for 'body-footer'}}
  </body>
</html>

run through...

// nodejs psuedocode...
const { preprocess, print } = require("@glimmer/syntax");
const path = require("path");

const fileContents = fs.readFileSync("index.html", { encoding: "utf8" });
const ast = preprocess(fileContents);
fs.writeFileSync(path.join("out", path.basename(hbsFile)), print(ast), { encoding: "utf8" });

becomes...

{{content-for "head"}}{{content-for "head-footer"}}{{content-for "body"}}{{content-for "body-footer"}}

and the AST looks like: https://gist.github.com/zimmi88/645a444f37066a58f25384713b7af822

I've tracked this to the doctype specifically because when I transform the first line into an HTML comment or remove it entirely, the output matches the input as expected.

Glimmer shouldn't usually come across doctype declarations, but supporting this might be useful for a couple of reasons...

  1. Completeness. If browsers can understand it, it makes sense that Glimmer would be able to understand it too.
  2. Glimmer's AST parser can reason about the index.html that ships with Ember CLI apps.

I hope this helps! Let me know if you need any additional details or information.

timlindvall avatar Dec 18 '18 21:12 timlindvall

FWIW, https://github.com/tildeio/simple-html-tokenizer/pull/71 is working on fixing this.

rwjblue avatar Apr 29 '20 18:04 rwjblue

This appears fixed by #1305. I did confirm by adding the above example into a local test, which passed.

courajs avatar Nov 17 '21 23:11 courajs

I don't think this has been fixed.

> require('@glimmer/syntax').preprocess('<!DOCTYPE html>')
{
  type: 'Template',
  body: [],
  blockParams: [],
  loc: SourceSpan {
    data: HbsSpan {
      source: [Source],
      hbsPositions: [Object],
      kind: 'HbsPosition',
      _charPosSpan: null,
      _providedHbsLoc: [Object]
    },
    isInvisible: false
  }
}

> require('@glimmer/syntax').print(require('@glimmer/syntax').preprocess('<!DOCTYPE html><div/>'))
'<div />'

fisker avatar Sep 08 '22 11:09 fisker

Any updates on this? This is quite a pain, and causes issues in projects such as Prettier.

Eejit43 avatar Aug 07 '23 02:08 Eejit43

I think it's worth supporting, but personally, i'm waiting until @wycats pr here: https://github.com/glimmerjs/glimmer-vm/pull/1428/files#diff-40f60e1037245d7b8a98a7325d53890a717da9979adeb54a61a795c4ba07f9c9

Is dealt with. Too many changes.

NullVoxPopuli avatar Aug 07 '23 11:08 NullVoxPopuli