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

Missing style output when globalStyle and layer rules are used together #1113

Open
2 tasks done
dadajam4 opened this issue Jun 5, 2023 · 4 comments
Open
2 tasks done
Labels
css Issue related to the core css package priority-high High priority issue

Comments

@dadajam4
Copy link

dadajam4 commented Jun 5, 2023

Describe the bug

When globalStyle and layer rules are used together in a single file, all but the last rule defined will be missing from the output.

import { globalStyle } from '@vanilla-extract/css';

export const a1 = globalStyle(':root', {
  '@layer': {
    testLayer: {
      color: 'red'
    }
  }
});

export const a2 = globalStyle(':root', {
  '@layer': {
    testLayer: {
      background: 'red'
    }
  }
});

Expected Output

@layer testLayer {
  :root {
    color: red;
    background: red;
  }
}

Actual output

@layer testLayer {
  :root {
    background: red; // color is missing.
  }
}

Reproduction

https://stackblitz.com/edit/stackblitz-starters-8jxr3t?description=Starter%20project%20for%20Node.js,%20a%20JavaScript%20runtime%20built%20on%20Chrome%27s%20V8%20JavaScript%20engine&file=issue.mjs,package.json&title=node.new%20Starter

System Info

System:
    OS: macOS 12.6
    CPU: (10) arm64 Apple M1 Pro
    Memory: 111.81 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.13.0 - ~/.anyenv/envs/nodenv/versions/16.13.0/bin/node
    Yarn: 1.22.17 - ~/.anyenv/envs/nodenv/versions/16.13.0/bin/yarn
    npm: 8.10.0 - ~/.anyenv/envs/nodenv/versions/16.13.0/bin/npm
  Browsers:
    Chrome: 113.0.5672.126
    Firefox: 105.0.3
    Safari: 16.0
  npmPackages:
    @vanilla-extract/css: ^1.11.0 => 1.11.0
    @vanilla-extract/esbuild-plugin: ^2.2.2 => 2.2.2
    @vanilla-extract/vite-plugin: ^3.8.2 => 3.8.2
    esbuild: ^0.17.19 => 0.17.19

Used Package Manager

pnpm

Logs

No response

Validations

@dadajam4
Copy link
Author

dadajam4 commented Jun 5, 2023

Supplementation. This reproduces not only the @layer rule, but also atrule like @media.

export const a1 = globalStyle(':root', {
  '@media': {
    all: {
      color: 'red'
    }
  }
});

export const a2 = globalStyle(':root', {
  '@media': {
    all: {
      background: 'red'
    }
  }
});

↓ output

@media all {
  :root {
    background: red;
  }
}

If a different selector is specified, the problem does not reproduce.

export const a1 = globalStyle(':root', {
  '@media': {
    all: {
      color: 'red'
    }
  }
});

export const a2 = globalStyle('.hoge', {
  '@media': {
    all: {
      background: 'red'
    }
  }
});

↓ output

@media all {
  :root {
    color: red;
  }
  .hoge {
    background: red;
  }
}

@dadajam4
Copy link
Author

dadajam4 commented Jun 6, 2023

*This is a comment that I am including in the hope that I can help in the investigation. Please forgive me.

https://github.com/vanilla-extract-css/vanilla-extract/blob/master/packages/integration/src/processVanillaFile.ts#L104-L110

  for (const [serialisedFileScope, fileScopeCss] of cssByFileScope) {
    const fileScope = parseFileScope(serialisedFileScope);
    const css = transformCss.transformCss({
      localClassNames: Array.from(localClassNames),
      composedClassLists,
      cssObjs: fileScopeCss
    }).join('\n');

The fileScopeCss variable at the beginning of this for statement appears to contain all the expected styles, but the return value of the following transformCss.transformCss method does not seem to contain the expected styles.

const css = transformCss.transformCss

@dadajam4
Copy link
Author

dadajam4 commented Jun 6, 2023

Then, when the following for statement looped multiple times, the style appeared to be missing due to processing of the subsequent root object.

https://github.com/vanilla-extract-css/vanilla-extract/blob/e88b6d8e15027757a5601e36b338acde757caa33/packages/css/src/transformCss.ts#L656C32-L658

export function transformCss({
  localClassNames,
  cssObjs,
  composedClassLists,
}: TransformCSSParams) {
  const stylesheet = new Stylesheet(localClassNames, composedClassLists);

  for (const root of cssObjs) {
    stylesheet.processCssObj(root);
  }

@dadajam4
Copy link
Author

dadajam4 commented Jun 6, 2023

Then I arrived at the code here.

https://github.com/vanilla-extract-css/vanilla-extract/blob/master/packages/css/src/transformCss.ts#L186-L195

    const activeConditionalRuleset =
      this.conditionalRulesets[this.conditionalRulesets.length - 1];

    if (
      !activeConditionalRuleset.mergeIfCompatible(this.currConditionalRuleset)
    ) {
      // Ruleset merge failed due to incompatibility. We now deopt by starting a fresh ConditionalRuleset
      this.conditionalRulesets.push(this.currConditionalRuleset);
    }

When the problem occurs, I think the mergeIfCompatible is terminating as expected because it does not enter this if statement, but it seems to be the merging process that is causing the problem.
Experimentally, I eliminated the if statement and the output of the styles is not pretty, but the CSS is as expected.

@mattcompiles mattcompiles added css Issue related to the core css package priority-high High priority issue and removed pending triage labels Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css Issue related to the core css package priority-high High priority issue
Projects
None yet
Development

No branches or pull requests

2 participants