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

Some styles missing when previewing or publishing project #107

Open
hobbes7878 opened this issue Sep 29, 2023 · 3 comments
Open

Some styles missing when previewing or publishing project #107

hobbes7878 opened this issue Sep 29, 2023 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@hobbes7878
Copy link
Member

hobbes7878 commented Sep 29, 2023

For anyone coming here, there appear to be some edge cases where a plugin we use to strip out unused style rules gets a bit greedy and can strip out some rules used in your custom svelte components. This likely only applies to styles applied through our utility class names, for example:

<p class="uppercase">lorem ipsum<p>

To get around this issue, you have a few different options:

  1. Use SCSS mixins instead of class names in the component whose styles are being stripped out.
<p>lorem ipsum</p>

<style lang="scss">
@import '@reuters-graphics/graphics-components/dist/scss/mixins';

p {
  @include uppercase;
}
</style>
  1. You can disable the plugin entirely by commenting out this line in vite.config.ts at the root of your project. Like this:
plugins: [
    sveltekit(),
    dsv(),
    svelteKitPagesPlugin(),
    // purgeCss(),
],
  1. Disabling the plugin will lead to slightly bloated stylesheets for your project. Alternatively, you can also use the plugin's safelisting config options to safe guard specific style rules individually. Like this:
plugins: [
    sveltekit(),
    dsv(),
    svelteKitPagesPlugin(),
    purgeCss({
      safelist: {
        standard: [ 'uppercase' ], // A specific class name
        greedy: [
          /^text-/, // Regular expressions can be used here to capture several classes
        ],
      },
    }),
],

📌 If you have this issue please add a comment here with a link to your project's repo. We're looking for more examples to help nail down why this sometimes happens...

@hobbes7878 hobbes7878 added the bug Something isn't working label Sep 29, 2023
@hobbes7878 hobbes7878 self-assigned this Sep 29, 2023
@hobbes7878
Copy link
Member Author

Repro TK, but @deaxmachina found that purgeCSS was stripping classes used in child components.

After closer inspection of our current plugin, it looks like this line is critical. I noticed the rollup docs say that isIncluded can be "null if external or not yet available". If I check that isExternal is not false within the same statement, the class passes through OK to the final stylesheet.

// if (info?.isIncluded !== true || info.code === null) continue;
if ((info?.isIncluded !== true && info?.isExternal !== false) || info.code === null) continue;

@hobbes7878
Copy link
Member Author

OK, after more digging, noticed the child component in question is fenced in by a (pretty normal) if condition that's only true after an element is measured in the browser. Basically:

<script>
  import Child from './Child.svelte';

  let containerWidth;
</script>

<div bind:clientWidth="{containerWidth}">
  {#if containerWidth}
    <Child />
  {/if}
</div>

That's apparently enough for vite/rollup to mark the module as not included in the graph (but also not external) at the point where we generate the bundle, thus not including its class names in the safelist.

If I give containerWidth an initial value (which we often don't do for perfectly reasonable layout shift reasons...), the module is marked included and styles are safeguarded.

That seems like enough of a general case to try to spin up a repro and ask the author if we can come up with a better condition to exclude modules.

That said, after playing with it, the only condition I could come up with to isolate this specific scenario was to specifically allow .svelte files not within node_modules to pass the condition... I'm sure that's not always good practice, though.

@hobbes7878
Copy link
Member Author

Hmm... I was unable to reproduce this on a fresh sveltekit install in: https://github.com/reuters-graphics/purgecss-repro

... with similar page/component structure and child nested under conditional statement that is only true in the browser. Unsure how this is an edge case at this point.

Will leave this open to collect any other examples and with the instructions for the nuclear option to shut the plugin down.

@hobbes7878 hobbes7878 changed the title PurgeCSS is stripping out classes used in child components Some styles missing when previewing or publishing project Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant