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

CSS modules: remove unused classes #5363

Merged
merged 11 commits into from
Jan 2, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 48 additions & 4 deletions flow-libs/postcss.js.flow
Expand Up @@ -8,17 +8,60 @@
import typeof {SourceMapGenerator} from 'source-map';

declare module 'postcss' {
declare type NodeCallback = (Node, number) => false | void;
declare type Callback<T> = (T, number) => false | void;

declare interface Input {
file?: string;
}

declare interface Position {
offset: number;
column: number;
line: number;
}

declare interface Source {
input: Input;
start?: Position;
end?: Position;
}

declare interface Node {
parent: Container;
type: 'atrule' | 'comment' | 'decl' | 'root' | 'rule';
source: Source;
+type: 'atrule' | 'comment' | 'decl' | 'root' | 'rule';
toJSON(): mixed;
}

declare interface Decl extends Container {
type: 'decl';
prop: string;
value: string;
source: Source;
}

declare interface Rule extends Container {
selector: string;
type: 'rule';
remove(): void;
}

declare interface AtRule extends Container {
name: string;
params: string;
type: 'atrule';
remove(): void;
}

declare interface Container extends Node {
each(callback: NodeCallback): false | void;
each(callback: Callback<Node>): false | void;
nodes: Array<Node>;
walk(callback: NodeCallback): false | void;
walk(callback: Callback<Node>): false | void;
walkRules(callback: Callback<Rule>): false | void;
walkDecls(nameFilter: string | RegExp, callback: Callback<Decl>): void;
walkDecls(callback: Callback<Decl>): void;
walkAtRules(nameFilter: string | RegExp, callback: Callback<AtRule>): void;
walkAtRules(callback: Callback<AtRule>): void;
}

declare interface Root extends Container {}
Expand Down Expand Up @@ -95,6 +138,7 @@ declare module 'postcss' {
comment: ($Shape<Node>) => Node,
decl: ($Shape<Node>) => Node,
rule: ($Shape<Node>) => Container,
fromJSON: <T: Node>(mixed) => T,
...
};
}
4 changes: 2 additions & 2 deletions packages/core/core/src/CommittedAsset.js
Expand Up @@ -119,9 +119,9 @@ export default class CommittedAsset {
return this.map;
}

getAST(): Promise<AST> {
getAST(): Promise<?AST> {
if (this.value.astKey == null) {
throw new Error('Asset does not have an AST');
return Promise.resolve(null);
}

if (this.ast == null) {
Expand Down
13 changes: 9 additions & 4 deletions packages/core/core/src/Transformation.js
Expand Up @@ -392,17 +392,22 @@ export default class Transformation {
}

// Make assets with ASTs generate unless they are js assets and target uses
// scope hoisting. This parallelizes generation and distributes work more
// evenly across workers than if one worker needed to generate all assets in
// a large bundle during packaging.
// scope hoisting or we do CSS modules tree shaking. This parallelizes generation
// and distributes work more evenly across workers than if one worker needed to
// generate all assets in a large bundle during packaging.
let generate = pipeline.generate;
if (generate != null) {
await Promise.all(
resultingAssets
.filter(
asset =>
asset.ast != null &&
!(asset.value.type === 'js' && asset.value.env.scopeHoist),
!(
(asset.value.env.scopeHoist && asset.value.type === 'js') ||
(this.options.mode === 'production' &&
asset.value.type === 'css' &&
asset.value.symbols)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😬

),
)
.map(async asset => {
if (asset.isASTDirty) {
Expand Down
8 changes: 4 additions & 4 deletions packages/core/integration-tests/package.json
Expand Up @@ -34,14 +34,14 @@
"lodash": "^4.17.15",
"marked": "^0.6.1",
"ncp": "^2.0.0",
"parcel": "2.0.0-beta.1",
"parcel-bundler": "2.0.0-beta.1",
"postcss": "^8.0.0",
"parcel": "2.0.0-beta.1",
"postcss-custom-properties": "^8.0.9",
"postcss-import": "^12.0.1",
"postcss-import": "^13.0.0",
Copy link
Member Author

@mischnic mischnic Dec 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One interesting situation that I ran into was that the old postcss-import version didn't use PostCSS 8 and inserted nodes into the AST that were missing some methods from recent my PostCSS PR (toJSON). This caused a build error

"postcss": "^8.2.1",
"posthtml-obfuscate": "^0.1.5",
"react": "^16.11.0",
"react-dom": "^16.11.0",
"react": "^16.11.0",
"rimraf": "^2.6.1",
"tempy": "^0.3.0",
"ws": "^7.0.0"
Expand Down
@@ -1,11 +1,15 @@
const postcss = require('postcss');

module.exports = postcss.plugin('postcss-test', () => (css, result) => {
css.walkRules(rule => {
rule.each(decl => {
if (decl.value === 'red') {
decl.value = 'green';
}
});
});
});
module.exports = (opts = {}) => {
return {
postcssPlugin: 'postcss-test',
Once(root, {result}) {
root.walkRules((rule) => {
rule.each((decl) => {
if (decl.value === 'red') {
decl.value = 'green';
}
});
});
},
};
};
module.exports.postcss = true;
@@ -0,0 +1 @@
{}
@@ -0,0 +1,3 @@
import styles from "./style.module.css";

output = styles["b-2"];
@@ -0,0 +1,11 @@
.unused {
color: red;
}

.b-2 {
font-size: 12pt;
}

:global(.page) {
border: 1px solid black;
}
@@ -0,0 +1,3 @@
body {
background: blue;
}
@@ -0,0 +1,4 @@
import "./global.css";
import * as styles from "./style.module.css";

output = styles["b-2"];
@@ -0,0 +1,3 @@
{

}
@@ -0,0 +1,11 @@
.unused {
color: red;
}

.b-2 {
font-size: 12pt;
}

:global(.page) {
border: 1px solid black;
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

83 changes: 81 additions & 2 deletions packages/core/integration-tests/test/postcss.js
Expand Up @@ -14,11 +14,12 @@ import {
NodePackageManager,
MockPackageInstaller,
} from '@parcel/package-manager';
import postcss from 'postcss';

describe('postcss', () => {
it('should support transforming css modules with postcss', async () => {
it('should support transforming css modules with postcss (require)', async () => {
let b = await bundle(
path.join(__dirname, '/integration/postcss-modules/index.js'),
path.join(__dirname, '/integration/postcss-modules-cjs/index.js'),
);

assertBundles(b, [
Expand All @@ -44,6 +45,84 @@ describe('postcss', () => {
assert(css.includes(`.${cssClass}`));
});

it('should support transforming css modules with postcss (import default)', async () => {
let b = await bundle(
path.join(
__dirname,
'/integration/postcss-modules-import-default/index.js',
),
{mode: 'production'},
);

assertBundles(b, [
{
name: 'index.js',
assets: ['index.js', 'style.module.css'],
},
{
name: 'index.css',
assets: ['style.module.css'],
},
]);

let output = await run(b);
assert(/_b-2_[0-9a-z]/.test(output));

let css = await outputFS.readFile(
b.getBundles().find(b => b.type === 'css').filePath,
'utf8',
);
let includedRules = new Set();
postcss.parse(css).walkRules(rule => {
includedRules.add(rule.selector);
});
assert(includedRules.has('.page'));
assert(includedRules.has(`.${output}`));
});

it('should tree shake unused css modules classes with a namespace import', async () => {
let b = await bundle(
path.join(
__dirname,
'/integration/postcss-modules-import-namespace/index.js',
),
{mode: 'production'},
);

assertBundles(b, [
{
name: 'index.js',
assets: ['index.js', 'style.module.css'],
},
{
name: 'index.css',
assets: ['global.css', 'style.module.css'],
},
]);

let js = await outputFS.readFile(
b.getBundles().find(b => b.type === 'js').filePath,
'utf8',
);
assert(!js.includes('unused'));

let output = await run(b);
assert(/_b-2_[0-9a-z]/.test(output));

let css = await outputFS.readFile(
b.getBundles().find(b => b.type === 'css').filePath,
'utf8',
);
let includedRules = new Set();
postcss.parse(css).walkRules(rule => {
includedRules.add(rule.selector);
});
assert.deepStrictEqual(
includedRules,
new Set(['body', `.${output}`, '.page']),
);
});

it('should support transforming with postcss twice with the same result', async () => {
let b = await bundle(
path.join(__dirname, '/integration/postcss-plugins/index.js'),
Expand Down