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

Leading whitespace should not be stripped from interpolated components #22

Closed
wchargin opened this issue Jul 27, 2018 · 2 comments
Closed

Leading whitespace should not be stripped from interpolated components #22

wchargin opened this issue Jul 27, 2018 · 2 comments
Labels
status: duplicate

Comments

@wchargin
Copy link

As currently written, this module performs interpolation before
stripping leading whitespace. In particular, whitespace is stripped from
the interpolated components. I think that this is the wrong behavior.

My understanding is that dedent is supposed to be effectively
syntactic sugar. Writing

function foo() {
  const things = dedent`\
    lorem ipsum
    dolor sit amet
  `;
  return things;
}

should be completely equivalent to writing

function foo() {
  const things = `\
lorem ipsum
dolor sit amet
`;
  return things;
}

with the sole benefit that the code indentation doesn’t have to become
wonky just to accommodate the text.

This suggests that

function foo() {
  const stuff = "abc\n    xyz";
  const things = dedent`\
    lorem ${stuff}
    dolor sit amet
  `;
  return things;
}

should also be equivalent to

function foo() {
  const stuff = "abc\n    xyz";
  const things = `\
lorem ${stuff}
dolor sit amet
`;
  return things;
}

which means that the output should be

lorem abc
    xyz
dolor sit amet

but with the current version of dedent the result is actually

lorem abc
xyz
dolor sit amet

Let me provide a concrete case where this matters. I was using dedent
to server-side render static pages on my site. The code looked roughly
like this:

            // lots of context, so this code is indented a bunch...
            const page = dedent`\
                <!DOCTYPE html>
                <html>
                <body>
                <div id="container">${html}</div>
                <script src="${pathToBundle}"></script>
                </body>
                </html>
                `;

This all worked well—I wrote it and forgot about it. But when I added
code blocks to my site, I noticed that they rendered incorrectly: code
like

<pre><code>function foo(bar, baz) {
    while (true) {
        if (bar()) {
            console.log("stuff");
            if (baz()) {
                console.log(
                    "even more stuff"
                );
            }
        }
    }
}</code></pre>

was being rendered as

<pre><code>function foo(bar, baz) {
while (true) {
if (bar()) {
console.log("stuff");
if (baz()) {
console.log(
    "even more stuff"
);
}
}
}
}</code></pre>

in the static HTML. Because pre sets white-space: pre, the resulting
code is rendered incorrectly in the browser, too. This was somewhat
difficult to track down!

I ended up replacing this module with a hand-written version that fit my
purposes better. (There was an additional change that I needed to make
to properly render code blocks with line-trailing backslashes.) You can
see that change here:

wchargin/wchargin.github.io@06475d4

Is this something that you’re interested in pulling upstream? I realize
that it is a breaking change, so I’d understand if you’re not
interested.

(Also, 👋 !)
@brandonkal
Copy link

brandonkal commented Jan 11, 2020 •
edited
Loading

Note that your function fails some tests:

it("should do nothing for escaped newlines in template tag", () => {
  const text = dedent`${"one "}\
    two
    three
  `;
  assertEquals(text, "one two\nthree\n");
});

The indent on the second line is never removed. Also the first line isn't handled specially.

This is why reading raw is the correct approach.

@JoshuaKGoldberg JoshuaKGoldberg added the status: in discussion Not yet ready for implementation or a pull request label May 22, 2023
@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue May 22, 2023
Closed
@JoshuaKGoldberg JoshuaKGoldberg added status: duplicate and removed status: in discussion Not yet ready for implementation or a pull request labels May 22, 2023
@JoshuaKGoldberg
Copy link
Collaborator

I think this is actually a duplicate of #12. Moving discussion there. Please let me know if that's wrong though!

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

No branches or pull requests
3 participants
@JoshuaKGoldberg @wchargin @brandonkal