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

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

Open
timlindvall opened this issue Dec 18, 2018 · 5 comments
Open

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

timlindvall opened this issue Dec 18, 2018 · 5 comments

Comments

@timlindvall
Copy link
Contributor

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 timlindvall changed the title AST preprocessing doesn't work with <!DOCTYPE> tags Preprocessing into AST doesn't work with <!DOCTYPE> tags Dec 18, 2018
@Turbo87 Turbo87 mentioned this issue Apr 29, 2020
Closed
@rwjblue
Copy link
Member

rwjblue commented Apr 29, 2020

FWIW, tildeio/simple-html-tokenizer#71 is working on fixing this.

@kangax kangax mentioned this issue Jun 22, 2020
Closed
@laurensclaessen laurensclaessen mentioned this issue May 12, 2021
Merged
@fisker fisker mentioned this issue Aug 4, 2021
Open
@courajs
Copy link
Contributor

courajs commented Nov 17, 2021

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

@fisker
Copy link
Contributor

fisker commented Sep 8, 2022

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 />'

@Eejit43
Copy link

Eejit43 commented Aug 7, 2023

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

@NullVoxPopuli
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@rwjblue @fisker @NullVoxPopuli @courajs @timlindvall @Eejit43