Strip <template> indentation by default

[NullVoxPopuli]

Propose stripping <template> indentation by default

Rendered

Summary

This pull request is proposing a new RFC.

To succeed, it will need to pass into the Exploring Stage, followed by the Accepted Stage.

A Proposed or Exploring RFC may also move to the Closed Stage if it is withdrawn by the author or if it is rejected by the Ember team. This requires an "FCP to Close" period.

An FCP is required before merging this PR to advance to Accepted.

Upon merging this PR, automation will open a draft PR for this RFC to move to the Ready for Released Stage.

Exploring Stage Description

This stage is entered when the Ember team believes the concept described in the RFC should be pursued, but the RFC may still need some more work, discussion, answers to open questions, and/or a champion before it can move to the next stage.

An RFC is moved into Exploring with consensus of the relevant teams. The relevant team expects to spend time helping to refine the proposal. The RFC remains a PR and will have an Exploring label applied.

An Exploring RFC that is successfully completed can move to Accepted with an FCP is required as in the existing process. It may also be moved to Closed with an FCP.

Accepted Stage Description

To move into the "accepted stage" the RFC must have complete prose and have successfully passed through an "FCP to Accept" period in which the community has weighed in and consensus has been achieved on the direction. The relevant teams believe that the proposal is well-specified and ready for implementation. The RFC has a champion within one of the relevant teams.

If there are unanswered questions, we have outlined them and expect that they will be answered before Ready for Release.

When the RFC is accepted, the PR will be merged, and automation will open a new PR to move the RFC to the Ready for Release stage. That PR should be used to track implementation progress and gain consensus to move to the next stage.

Checklist to move to Exploring

• The team believes the concepts described in the RFC should be pursued.
• The label S-Proposed is removed from the PR and the label S-Exploring is added.
• The Ember team is willing to work on the proposal to get it to Accepted

Checklist to move to Accepted

• This PR has had the Final Comment Period label has been added to start the FCP
• The RFC is announced in #news-and-announcements in the Ember Discord.
• The RFC has complete prose, is well-specified and ready for implementation.
  • All sections of the RFC are filled out.
  • Any unanswered questions are outlined and expected to be answered before Ready for Release.
  • "How we teach this?" is sufficiently filled out.
• The RFC has a champion within one of the relevant teams.
• The RFC has consensus after the FCP period.

[NullVoxPopuli]

I should link to the documentation that proves the browser invisible character handling behavior, and provide a jsbin proving it

[NullVoxPopuli]

update this to focus on the stripIndent / common-tags functionality by default.

as a stretch goal, maybe <template minify> for removing all "extraneous" invisible characters in a component.

[void-mAlex]

you have to be careful about <pre> blocks
which I guess are in a weird state already due to increased indentation from a migration

[NullVoxPopuli]

Yea pre blocks aren't affected here

[void-mAlex]

but they might be by the stripping of the indentation

[NullVoxPopuli]

do you have an example where something would be problematic?

The way I see it:

<template>
  <pre></pre> <- no indentation to strip within the pre
</template>

<template>
  <pre>

  </pre> <- the indentation to strip doesn't matter
</template>  

<template>
  <pre>
</pre> <- indentation inconsistent, no stripping occurs
</template>

[void-mAlex]

in your second example the indentation between the open and close tags matters and is reproduced by the browser exactly
it would be incorrect to touch it

<template>
  <pre>\fake tab char \fake tab char something something
aaa
  </pre>
</template>  

indentation outside pre should be stripped even though pre doesn't align with the whitespace count of outside of it

[NullVoxPopuli]

indentation outside pre should be stripped even though pre doesn't align with the whitespace count of outside of it

I don't agree -- it would mean we have to parse the template contents to determine if we are in a pre tag and have a different set of rules.

if someone is going to put their pre-indentation all the way to the left like that, the whole template gets no indentation, per:

indentation is determined by the smallest number of invisible characters for each line

We could later change that, but I don't want implementation to hard.
as is, implementation would just be line-by-line iteration -- no parsing of the contents needed.

[void-mAlex]

I don't agree -- it would mean we have to parse the template contents to determine if we are in a pre tag and have a different set of rules.

but you have to do that anyway as it would be a bug to strip whitespace from a pre tag even if the indentation rule passes the check

[MelSumner]

@NullVoxPopuli reminder for myself (and for you to bug me if I am too slow) to link to places where whitespace has an effect on a11y outcomes.

[NullVoxPopuli]

is that different from the already referenced docs?:

• from MDN
• from CSS WG: White Space Processing & Control Characters
• from CSS WG: Collapsible White Space

[ef4]

Discussion notes from RFC review:

• this feature seems reasonable, making the indentation of template tags affect their content is not nice.
• one can argue this is a bug fix, but we may want to strategize how to communicate about this when the change rolls out. A typical app will not have many places where this matters, but it might have a few and communicating that to developers is the challenge.

[void-mAlex]

maybe having an attribute on the template tag itself to disable the white space stripping would negate the drawback?

[NullVoxPopuli]

Personally, I'd prefer if we do add an attribute on the <template>, that it would aggressively strip invisible characters, so like <template minify> would remove all extraneous (when not using white-space:pre) invisible characters -- which is what ember-hbs-minifier does.

[void-mAlex]

I see your minify and raise you <template minify> current proposed version that could be ommited OR <template minify="off"> OR <template minify="full">

[NullVoxPopuli]

Do you think it's worth the drawback of doing that: that everyone who migrated to gjs/gts who now has a ton of invisible characters doesn't get them removed ever unless they change all of their components (again)?

Regarding minify="off", I think I could vibe with this, but more as <template preserve-indentation> (no value, is clear about what it does (because it's not really the opposite of what I think minify should do))

I think all of this sounds reasonable as a subsequent RFC -- tho preserve-indentation could make sense in this RFC, tho has wider reaching impacts on syntax highlighting implementations as it would be our first <template> attribute

[void-mAlex]

I think all of this sounds reasonable as a subsequent RFC -- tho preserve-indentation could make sense in this RFC, tho has wider reaching impacts on syntax highlighting implementations as it would be our first <template> attribute

this sounds like less of rfc and more like this is what you will get

I still strongly believe that at a minimum we should ship a preserve-indentation attribute on template tag as part of this rfc as it would pretty much resolve the drawback stated and provide a way forward to those that are affected

would be nice to have a global optout as well but I recognise that a lot of damage may have already been caused by moving to the gjs file format

[ef4]

Notes from RFC review discussion including feedback to @void-mAlex's comments above:

The design does offer an opt-out, because only whitespace that is on every single line will be stripped, so you can prevent all de-indentation like:

    <template>
{{!-- prevent automatic de-indent --}}
       pre content here
    </template>

[NullVoxPopuli]

Notes from RFC review discussion

The RFC has been updated to include the outputs of this discussion.

Going to merge this RFC as FCP is passed.
Thanks everyone!

[NullVoxPopuli]

Implementation: embroider-build/content-tag#112