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

Should we trim whitespace #7

Closed
straker opened this issue Feb 20, 2016 · 12 comments
Closed

Should we trim whitespace #7

straker opened this issue Feb 20, 2016 · 12 comments

Comments

@straker
Copy link
Owner

straker commented Feb 20, 2016

New line characters at the start and end of the html string create text nodes. Should we just trim the string to remove these? I don't see any reason you would purposefully want those start and end text nodes myself.

// creates an empty text node, a p tag, and an empty text node
html`
<p>foo</p>
`;
@straker straker changed the title Should we trim whitespace? Should we trim whitespace Feb 20, 2016
@jshcrowthe
Copy link
Collaborator

I agree. I don't think that anyone would intentionally want to create an empty text node before the element they are actually creating. trim'ing the passed string before generating the HTML is probably smart.

@tbranyen
Copy link

Definitely, for example:

function render() {
  document.querySelector('main').diffElement(html`
    <main>
      <h1>${Date.now()}</h1>
    </main>
  `);

  requestAnimationFrame(render);
}

render();

In order to make this example work as one would expect (diffing the same element structure), I'd need to manually trim the pre/post whitespace.

@domenic
Copy link

domenic commented Feb 22, 2016

I'm wary of this. I think this kind of magic is generally not something you want in a standards-based library. Every template string tag has this "problem" and the canonical solution is to just put your ```s snuggled up close to the content.

It also seems counterintuitive that if I do something like html - `` meaning explicitly to create a Text node with whitespace on either side, you'll break that and create one with - as its contents.

@annevk, what do you think?

@annevk
Copy link

annevk commented Feb 22, 2016

I agree that would be bad. It should be as close as possible to the HTML parser, possibly the <template> parser. That is, if we ever end up standardizing this, that's what we'd want to put it on top on, coupled with the appropriate mode switches.

@jshcrowthe
Copy link
Collaborator

I guess I can get behind that because the intent is that this is adopted by the DOM API. I very rarely see users intentionally creating empty text nodes and trim'ing the passed input is a solution to solve that (albeit a naive one as is evidenced by @domenic's example).

So assuming that we don't trim the input, are there valid use cases for truly empty text nodes that I'm just not aware of? As discussed in #6 if we are going to adopt DocumentFragment as our return type then largely this goes away but just as a point of clarity (for myself).

@domenic
Copy link

domenic commented Feb 22, 2016

I know Ember uses empty text nodes as "markers" for data-binding positions within the DOM. Not sure it's terribly relevant to this though :)

@straker
Copy link
Owner Author

straker commented Feb 23, 2016

I think we all agree that we shouldn't make any assumptions about what the user's intent was, so I'll close this issue with no action and default everything to the HTML parser.

@straker straker closed this as completed Feb 23, 2016
@cmcaine
Copy link
Contributor

cmcaine commented Nov 21, 2017

This has just bitten me when trying to use newlines. Without whitespace getting trimmed it's quite difficult to use this function without making a real mess of your code.

Unless there's some trick I'm missing, this decision makes the library pretty much unusable for me.

For reference, my code:

        this.html = html`<div>
            <span>${pre}</span>
            <img src=${favIconUrl}></img>
            <span>${tab.index + 1}: ${tab.title}</span>
            <a class="url" href=${tab.url}>${tab.url}</a>
        </div>`

I encourage you to revisit this decision - we all want our code to be easy to read, after all.

@straker
Copy link
Owner Author

straker commented Nov 21, 2017

Could you please describe what your expected output was? Running the provided code through html produces what I would expect (as in, I don't see any text nodes being created due to whitespace not being trimmed at the start or end).

@cmcaine
Copy link
Contributor

cmcaine commented Nov 21, 2017

Certainly :)

I expected the input to be trimmed linewise.

Concretely:

    html`<div>
            <span></span>
            <img src=foo></img>
            <span>: </span>
            <a class="url" href=foo></a>
        </div>`

gives a node with 9 children. I expected the four obvious ones, I didn't expect the five text nodes that are interleaved.

@straker
Copy link
Owner Author

straker commented Nov 21, 2017

I see. So this issue specifically asked about trimming whitespace off the very front of the string and the very end usingString.prototype.trim(). The text nodes you see interlaced between the DOM nodes would happen even if you put it into a .html file due to the HTML template parser. We decided to just default to the HTML parser since that would keep it consistent with what would be expected if you wrote that in an HTML file for readability.

@cmcaine
Copy link
Contributor

cmcaine commented Nov 21, 2017

Heh, I got confused because someone had set white-space: pre for a containing element.

Sorry about that. Yes, this is fine. I retract my comments :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants