[QUEST] Glimmer

[tchak]

Objective

We want to get Prettier’s support for Ember templates up to the point where it is complete and maintainable enough to be officially released.

This quest issue was created to track the outstanding work required to meet this objective and to help volunteers coordinate to get the work done.

Contributing

You can follow and participate in the work on the collaborative issue

[tchak]

Maybe I missed some discussion/context. In which case you can point me to some reading :)

[ikatyang]

We can officially release it once it's able to prettier all the supported syntaxes (i.e., every cases mentioned in its spec or its official test cases), I'm not familiar with handlebars so I'm not sure if it's ready.

[tchak]

It's not :)
I will continue to fix obvious missing syntax and will try to find what would be the official reference.

[tchak]

@rwjblue what is the most up to date document with all the Glimmer syntaxes?

Also, do you have any idea on why {{collection.[2]}} is parsed as {{collection.2}} and I have no hint in the AST about the original form?

[Alonski]

@tchak Can anything be done to help get all the issues for this fixed?

[ghost]

On a related note, I brought up the above syntax in discord and it apparently shouldn't work in Ember templates: ember-template-lint/ember-cli-template-lint#437

[tchak]

@Alonski I think the best thing you can do is to try it and report any issues/suggestions in here. I will remove theArray paths issue problem from the list as it appears to not be officially supported and it is a pain to implement.

[Alonski]

@tchak So it seems that maybe this is more ready than we though? I am in the process of getting my dev team to start using it and to autofix all templates.

Maybe all that is missing is some tests around various templates before/after?

[tchak]

I think some new stuff (like modifiers) was added to glimmer since last time I looked and would need to be checked.

[duailibe]

@GavinJoyce is working through some changes, if you want to collaborate.

[jgwhite]

@tchak @GavinJoyce et al. we (Heroku) are also motivated to improve Glimmer support in Prettier. Shall we try to coordinate our efforts somehow? Quest issue? ⚔️ 😅

[GavinJoyce]

@jgwhite nice one!

A quest issue would be good, there are certainly some currently actionable items and I know that many people in Intercom have also expressed an interest in helping out too.

I believe that there are some changes required in @glimmer to make the AST lossless, here's one that I know of:

• https://github.com/rajasegar/ember-angle-brackets-codemod/issues/46#issuecomment-487108829

@rwjblue, perhaps you know of others?

In addition to this, cataloging and possibly fixing issues with the current prettier hbs printer would be really useful.

[ghost]

@Alonski @mfeckie and I conversed a few months back about this and Martin documented some whitespace issues here: #5340.
A quest with an updated catalog would help greatly to push this over the line. I can also help test on a largish ember application.

[GavinJoyce]

While we're working on improving glimmer support, here's a little snippet to add a glimmer option to https://prettier.io/playground/:

let select = document.querySelectorAll('select')[0];
let option = document.createElement("option");
option.text = "glimmer";
select.append(option);

[jgwhite]

@tchak we could potentially re-purpose this issue as a quest issue. What do you think? (assuming the prettier team are OK with the increased noise)

[GavinJoyce]

I wonder if there's a particular discord server channel that we should use to coordinate our efforts? We could create #dev-prettier perhaps?

[duailibe]

@tchak we could potentially re-purpose this issue as a quest issue. What do you think? (assuming the prettier team are OK with the increased noise)

No problem with that

I wonder if there's a particular discord server channel that we should use to coordinate our efforts? We could create #dev-prettier perhaps?

This sounds good. We used to have something in "Gitter", but never took off, I think. Feel free to PR changes advertising it in docs/README/CONTRIBUTING/etc if you do it

[Alonski]

@GavinJoyce I am working on getting a #st-tooling or #topic-tooling channel on the Ember Discord.

Currently, what we have available is #topic-editors. You are warmly welcome :)

[tchak]

@jgwhite sure. It was kinda my intention when I created it.

[jgwhite]

@tchak wonderful. I started drafting the quest here if it’s helpful: jgwhite#1

[lifeart]

one more issue: trying to format

{{#let (query-node "#exercise-steps-slot") as |node|}}
  {{#if node}}
    {{#in-element node}}
      {{#unless this.justEnteredTask}}
        <ExerciseSteps
          @activeStep={{this.mode}}
          @onClick={{this.onModeChange}}
        />
      {{/unless}}
    {{/in-element}}
  {{/if}}
{{/let}}

getting error and:

{{#let (query-node "#exercise-steps-slot") as |node|}}
  {{#if node}}
    {{#in-element node guid="%cursor:0%" nextSibling=null}}
      {{#unless this.justEnteredTask}}
        <ExerciseSteps
          @activeStep={{this.mode}}
          @onClick={{this.onModeChange}}
        />
      {{/unless}}
    {{/in-element}}
  {{/if}}
{{/let}}

{{#in-element node guid="%cursor:0%" nextSibling=null}}

playground

[rwjblue]

@lifeart - That issue was fixed in glimmerjs/glimmer-vm#1086 and is included in @glimmer/syntax 0.52.1+ ( prettier@2.0.5 is using 0.50.0, but master is on 0.55.1).

[boris-petrov]

I tried Prettier 2.1.0 and there are a few issues that I encountered. Both can be seen in this playground link.

1. {{~!~}} is transformed to {{!}} which is wrong.

2. htmlWhitespaceSensitivity: 'strict' is not followed. That is, I can't find a way to tell Prettier for Handlebars to treat whitespace as sensitive. I don't want:

<a>foobar</a>
barbaz

To become:

<a>
  foobar
</a>
barbaz

Because that leads to a different HTML output (the link contains a whitespace at the end). Changing the parser in the playground link to html fixes the issue.

Are these going to be fixed? Is there anything I can do about the second issue right now?

[alexander-akait]

@boris-petrov PR welcome, 1. should be easy to solve, 2. is more hard, but you can find a lot of example for HTML

[kangax]

@boris-petrov whitespace issue is known and is not trivial #8527 Probably the only major bug before glimmer could be considered stable in Prettier. Is {{~!~}} a whitespace-sensitive HBS comment? Tilde handling was fixed in #7575 but maybe there's a bug where it doesn't work with comments?

[boris-petrov]

@kangax - I see. The whitespace issue is pretty much a show-stopper for any real project so until then, Prettier is unusable unfortunately. Hopefully someone will come up with a solution.

As for {{~!~}} - that's a whitespace-removing HBS comment. So:

<div>
  {{~!~}}
  <span></span>
  {{~!~}}
</div>

Is the same as:

<div><span></span></div>

Note that {{~!}} and {{!~}} also don't work correctly.

[kangax]

@boris-petrov I talked a little bit about possible solutions in the ticket for the whitespace issue. I'd love to hear your feedback there. Haven't had time to work on it lately, unfortunately.

As far as {{~!~}}, one solution is to strip those comments and print out an output that has no whitespace. Another option (much easier) is to at least leave those comments untouched. Currently, glimmer printer in Prettier always outputs {{! for any comment unless there's {{ inside, in which case it understands that this is a multi-line comment and so it adds -- around 833666a#diff-eb371de501a3e14eb500ea3481e34cceR189 I see that code hasn't been touched since it was added almost 3 years ago. As you can see, that's also why neither {{~ nor ~}} work.

[boris-petrov]

@kangax - I added a comment on the other issue.

As for {{~!~}} - I think the best thing to do is for sure to just leave the comments (and the code) unformatted. The reason I've put this comment is to visually have something on a new line (for readability) but to semantically have no whitespace - so I want both of those preserved in the output. If that's also the easiest way to implement - all the better. 😄

By the way, I also forgot a third issue. Prettier actually blew up on: {{someArray.[0]}}. I had to change it to {{someArray.firstObject}} for the parser to not throw an exception but that doesn't work in all cases (mine included) when someArray isn't an EmberArray but a plain JS array.

[kangax]

@boris-petrov made a PR (#9082) for the tilde issue, just need to add a changelog file there if core team agrees this is an acceptable approach.

[kangax]

@boris-petrov where can I find more information on {{someArray.[0]}} — first time I see this (been working w. Ember for a few months only), why is this syntax even a thing? I see that it's changed to .0 is that what the issue is?

[chrisrng]

^ That {{someArray.[0]}} syntax is actually invalid but works

From discord I guess the closest thing to this being documented is this issue: ember-template-lint/ember-template-lint#787

There is https://github.com/mahirshah/ember-array-index-template-codemod to migrate your code

[boris-petrov]

Oh, I didn't know that it's invalid. 😄 Thanks for pointing that out @chrisrng

@kangax - it's strange that the playground "works". What I've done is I've installed ember-template-lint-plugin-prettier and I run ember-template-lint . --fix which leads to:

(node:102603) UnhandledPromiseRejectionWarning: Error: Parse error on line 42:
...ons")) this.objects.0)        }}
-----------------------^
Expecting 'ID', got 'NUMBER'
    at Parser.parseError (/home/boris/project/node_modules/handlebars/dist/cjs/handlebars/compiler/parser.js:267:19)
    at Parser.parse (/home/boris/project/node_modules/handlebars/dist/cjs/handlebars/compiler/parser.js:336:30)
    at HandlebarsEnvironment.parseWithoutProcessing (/home/boris/project/node_modules/handlebars/dist/cjs/handlebars/compiler/base.js:46:33)
    at preprocess (/home/boris/project/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser/tokenizer-event-handlers.js:333:22)
    at new ParseResult (/home/boris/project/node_modules/ember-template-recast/src/parse-result.js:90:15)
    at Object.parse (/home/boris/project/node_modules/ember-template-recast/src/index.js:8:16)
    at Prettier.exit (/home/boris/project/node_modules/ember-template-lint-plugin-prettier/lib/rules/prettier.js:103:34)
    at /home/boris/project/node_modules/ember-template-lint/lib/rules/base.js:297:21
    at visitNode (/home/boris/project/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/traversal/traverse.js:111:16)
    at traverse (/home/boris/project/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/traversal/traverse.js:219:3)
(Use `node --trace-warnings ...` to show where the warning was created)

The code is actually this.objects.[0] - not sure why in the error message it has removed []. In any case, if that's unsupported syntax, no need to worry about it.

[chriskrycho]

👋🏼 can we get a release cut that has all the features we've landed for Glimmer lately included (totally fine to leave them still in "beta" mode for now)? That'd help unblock a major effort at LinkedIn.

[thorn0]

If it's really urgent, just install Prettier directly from GitHub. Prettier supports that: yarn add prettier/prettier

[chriskrycho]

@thorn0 Yep, thank you for raising it in case we didn't know, but in this case I'm very well aware! That's a great solution in many contexts, but in many large corporate contexts it's not easily possible: for example, if all your packages need to be re-hosted on an internal repo, in which case it needs to actually be published. (We can work around that temporarily, but with difficulty at best.)