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

Tree Shakeable Injector #122

Closed
garand opened this issue Jun 18, 2020 · 13 comments
Closed

Tree Shakeable Injector #122

garand opened this issue Jun 18, 2020 · 13 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@garand
Copy link

garand commented Jun 18, 2020

Hey! Love the project, working great for me. However I'd like to be able to tree-shake my styles with my components. For instance, if I had 10 components, all 10 groups of styles would be injected even if only 3 components are used on a page.

I like the approach here where

<div className={style.myThing} />

Becomes

<div className={style().myThing} />

Out of the box that package doesn't work with rollup-plugin-styles. I'm trying to experiment with a custom injector to do it, but no luck just yet.

Is this something you'd consider adding as a built in option?

@Anidetrix
Copy link
Owner

Anidetrix commented Jun 18, 2020

Hi @garand,

If your mean class exports - at the time, object treeshaking is not yet supported by rollup (see here for details), but there are alternative solutions for this:

  1. Use named exports
    or
  2. Use rollup-plugin-terser

If you are talking about stripping unused CSS - consider code splitting using dynamic imports or manual chunks, or try PurgeCSS.

As of injecting only parts of styles - does it even make sense since the CSS is already bundled inside JS loaded into browser? (EDIT: See the comment below)

Please inform me if you have an argument about why it should be different or if I got something wrong.

Hope it helps!

@Anidetrix
Copy link
Owner

Anidetrix commented Jun 18, 2020

Looking through the provided plugin again, I think I now understand what this accomplishes. By the way, it is not connected to injector, but to generated embedding code. Shouldn't be too hard to implement actually, so I'll try.

Also, I think using object getters/setters would be a better approach, don't you think? It would not cause any breaking changes and will keep syntax clean.

UPDATE 1: Seems this will also require an additional option since with this approach CSS won't be injected at all, even the global one, unless classes are referenced inside code. Also it probably won't be compatible with named exports unless they are wrapped in individual functions. So if anyone knows a better solution I'm all ears!

@Anidetrix Anidetrix self-assigned this Jun 19, 2020
@Anidetrix Anidetrix added enhancement New feature or request help wanted Extra attention is needed labels Jun 19, 2020
@Anidetrix
Copy link
Owner

Anidetrix commented Jun 26, 2020

Hi again @garand,

This feature is available now, in version 3.10.0! To enable it use treeshakeable option:

styles({ mode: ["inject", {treeshakeable: true}] })

I gave myself a week to come up with something better, but in the end getter-based approach seems like the best compromise for now. Also, to inject global styles without using any classes, use it like so:

import styles from "./style.css"
styles.inject()

Let me know if that works for you!

@garand
Copy link
Author

garand commented Jun 26, 2020

@Anidetrix This is awesome! I'll try it out right now. Do I need to run .inject() or can I call it like I referenced in the initial issue comment?

@Anidetrix
Copy link
Owner

@garand Use .inject() only if you don't use any classes from the file and you still want it to be injected.

If you use any class, like so:

import styles from "./style.css"
console.log(styles.anyClassName)

CSS will be injected automatically.

@garand
Copy link
Author

garand commented Jun 26, 2020

Great! I don't see 3.10.0 yet, is that still being deployed?

Also... all of a sudden I am getting this error recently:

[!] (plugin styles) Error: Unable to load PostCSS plugin `0`

Not sure what would be causing it. This is my postcss.config.js. That config is working fine in other projects and all of the dependencies are installed.

module.exports = {
  plugins: [
    require('postcss-import'),
    require('postcss-edgie'),
    require('tailwindcss'),
    require('postcss-preset-env')({ stage: 0 }),
    require('autoprefixer'),
  ],
}

I can create a separate issue if needed.

@Anidetrix
Copy link
Owner

Great! I don't see 3.10.0 yet, is that still being deployed?

Yes

[!] (plugin styles) Error: Unable to load PostCSS plugin 0

This was also fixed in 3.10.0

@Anidetrix
Copy link
Owner

3.10.0 is up if you haven't seen yet, hope this is what you were looking for!

@garand
Copy link
Author

garand commented Jun 26, 2020

Already installed and trying it now. I'll let you know if it's doing what I expect!

@garand
Copy link
Author

garand commented Jun 26, 2020

Seems to be working just great so far. Will make additional issues later if I discover any functionality that is missing. Thanks for the hard work. 🙌

@garand garand closed this as completed Jun 26, 2020
@garand
Copy link
Author

garand commented Jun 26, 2020

@Anidetrix For that PostCSS plugin issue... it came up again, I cleared out node_modules, installed again, and then it built fine... Any ideas? Seems to come up periodically.

@Anidetrix
Copy link
Owner

@garand I believe this should not be the case since code for that isn't particularly complex, probably package manager just messed up. But if the issue will appear on a regular basis I can investigate more thoroughly.

@garand
Copy link
Author

garand commented Jun 26, 2020

Okay, sounds good. I'll let you know if it keeps coming up. Appreciate the quick responses!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants