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

addFileScope: Don't mangle non-template strings that contain newlines #1406

Merged
merged 1 commit into from
May 4, 2024

Conversation

askoufis
Copy link
Contributor

@askoufis askoufis commented May 4, 2024

Fixes #1405.

TL;DR: multi-line strings with literal newlines were being manged by dedent (ultimately because of SWC, see extra notes below).

These used to be correctly dedented by outdent in @vanilla-extract/integration@7.1.2, but since swapping to dedent, it was no longer being handled correctly.

The fix is simple though: call dedent as a function, rather than a string tag. This was actually a bug in dedent that was fixed relatively recently, hence the version bump.

Extra notes

I was stumped for a while trying to reproduce this within VE's test suite. I was seeing identical output between dedent and outdent. I then realised that there are actually two loaders being used in the reproduction: Vanilla Extract's, and the swc loader.

It turns out that swc targets ES3 syntax by default. This results in all template strings being converted to regular strings, because template strings were only added in ES6. In the case of multi-line strings, you end up with something like:

// input
const multiLineString = `
    foo
`;

// SWC ES3 output (eventually mangled by dedent)
var multiLineString = "\n   foo\n";

So an alternative way to fix this is to configure storybook to either not use SWC, or configure SWC to target ES6 or greater:

// .storybook/main.js
const config = {
  swc: (config) => {
    return {
      ...config,
      jsc: {
        ...config.jsc,
        target: "es6",
      },
    };
  },
}

Obviously fixing the issue within VE is the best option, but I'm glad I'm at least aware of potential Storybook + SWC issues going forward.

Copy link

changeset-bot bot commented May 4, 2024

🦋 Changeset detected

Latest commit: 4d5f348

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@vanilla-extract/integration Patch
@vanilla-extract-private/tests Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@askoufis askoufis merged commit 5e06ada into master May 4, 2024
11 checks passed
@askoufis askoufis deleted the preserve-escaped-newlines branch May 4, 2024 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multline strings + webpack + storybook gives syntax error
1 participant