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

Dead code elimination #13

Open
rtsao opened this issue Jul 10, 2015 · 4 comments
Open

Dead code elimination #13

rtsao opened this issue Jul 10, 2015 · 4 comments

Comments

@rtsao
Copy link
Contributor

rtsao commented Jul 10, 2015

Would it be possible to not output styles that aren't used when composing classes?

For example:

main.css

.foo {
  background: purple;
  composes: abc from "./other.css";
}

other.css

.abc {
  color: blue;
}
.xyz { /* note: unused */
  border: 10px solid red;
}

main.js

var styles = require('./main.css');

output css

.PyjhzcC6jE { /* .foo */
  background: purple;
}
.xGcJd4AVqt { /* .abc */
  color: blue;
}

See also this failing test: #12

This functionality would be great when composing just a single class from a large css file. I'd be interested in helping implement this.

@rtsao rtsao changed the title Don't process unused styles Don't output unused styles from composed stylesheets Jul 10, 2015
@geelen geelen changed the title Don't output unused styles from composed stylesheets Dead code elimination Jul 11, 2015
@geelen
Copy link
Member

geelen commented Jul 11, 2015

Hey, this is a great idea, and we've been talking about it a bit, so I wanted to change the title and kick off the discussion.

So, this is really hard. Right now, each file gets processed independently, so no file knows which parts of it are being used. If we change that, we have some problems:

  • The file that's required from JS probably needs to be included 100%, since you won't know which exported classes are actually being used unless you run through every part of your app
  • If a file is used in multiple places, things get complex. Currently, the first time a file is referenced, it gets injected 100% into the output and the exports are cached. The next time it's referenced, you simply return the cached outputs. That gets tricky if you're only injecting the classes that are used.

So, theoretically I can see a mechanism where each file has sort of a line-by-line shadow map, where files are injected into some kind of intermediate area, and each time a class is referenced those lines are "illuminated", then at the "end" of the process (another tricky concept) you export all the illuminated lines and you have your file. BUT I kind of feel that's more trouble than its worth.

I think a smarter approach is something like uncss where you look at the output of a file and remove everything unused. Uncss is super complex because it has to deal with real-world global selectors by firing up PhantomJS and jumping around your site. I think with CSS Modules we have the ability to do something better. Perhaps a command-line utility that you point at your "entry" CSS files, walks the tree and accumulates all the exported classes that you use, then looks through your bundled CSS and deletes anything you're not exporting?

Anyway, yes I think we should do this... somehow :)

@rtsao
Copy link
Contributor Author

rtsao commented Jul 11, 2015

Perhaps I'm thinking of something more rudimentary. It would be difficult to eliminate styles that are required but not actually used (akin to uncss approach), but what about simply eliminating classes that are not part of the "composes" tree?

For example we have (taken from here https://github.com/css-modules/postcss-modules-extract-imports/tree/master/test/test-cases/import-multiple-classes):

:local(.exportName) {
  composes: importName secondImport from 'path/library.css';
  other: rule;
}

which becomes:

:import("path/library.css") {
  i__imported_importName_0: importName;
  i__imported_secondImport_1: secondImport;
}
:local(.exportName) {
  composes: i__imported_importName_0 i__imported_secondImport_1;
  other: rule;
}

When we process the :import("path/library.css") statement, wouldn't it be possible to filter out everything other than .importName and .secondImport from path/library.css before doing anything.

Forgive my unfamiliarity with the actual implementation, I'm still digging through the source.

@geelen
Copy link
Member

geelen commented Jul 11, 2015

Yeah, you're on the right track I think, but if path/library.css is used in multiple places, you need to keep the rest of the rules around just in case they're used later.

But, once you have the full tree, you can go back over the generated CSS and delete stuff that's not present in it, for sure.

@rtsao
Copy link
Contributor Author

rtsao commented Aug 22, 2015

I've implemented this here: #20

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 a pull request may close this issue.

2 participants