Skip to content

Commit

Permalink
refactor: Determine CSS module by filename, not directory (#1714)
Browse files Browse the repository at this point in the history
* refactor: Determine CSS module by filename, not directory

* docs: Adding changeset

* refactor: Project creation pull from templates 'main' branch

* test: Updating tests

* revert: Accidentally removed log message
  • Loading branch information
rschristian committed Dec 20, 2022
1 parent 2e6f58a commit 79dcc3a
Show file tree
Hide file tree
Showing 23 changed files with 115 additions and 103 deletions.
11 changes: 11 additions & 0 deletions .changeset/cyan-tomatoes-count.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
'preact-cli': major
---

Alters CSS Module detection to instead rely upon file names, rather than directory names.

Treating all CSS files found within `routes/` and `components/` as CSS Modules was not obvious, nor did it offer an easy way to opt out (or in) without editing the Webpack config itself.

This change makes is so that users can opt into CSS Modules from anywhere in their app by instead naming their CSS files according to the pattern `*.module.css`.

Anyone using CSS Modules within `routes/` or `components/` will need to alter their CSS files to be `x.module.css`. If you've disabled CSS Modules in your `preact.config.js`, you can remove that bit of configuration and use file names to instead determine behavior.
13 changes: 13 additions & 0 deletions packages/cli/global.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,16 @@ declare module 'shelljs' {
};
export = shell;
}

declare module '*.module.css' {
const classes: { [key: string]: string };
export default classes;
}
declare module '*.module.sass' {
const classes: { [key: string]: string };
export default classes;
}
declare module '*.module.scss' {
const classes: { [key: string]: string };
export default classes;
}
5 changes: 5 additions & 0 deletions packages/cli/src/commands/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,11 @@ exports.create = async function createCommand(repo, dest, argv) {
if (!repo.includes('/')) {
repo = `${ORG}/${repo}`;
info(`Assuming you meant ${repo}...`);

// TODO: Remove this after updating all templates
if (repo.endsWith('default') || repo.endsWith('typescript')) {
repo += '#next';
}
}

if (!existsSync(resolve(cwd, dest, 'src'))) {
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/lib/webpack/run-webpack.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ function writeJsonStats(cwd, stats) {
function allFields(stats, field, fields = [], name = null) {
const info = stats.toJson({
errors: true,
warnings: false,
warnings: true,
errorDetails: false,
});
const addCompilerPrefix = msg =>
Expand Down
41 changes: 19 additions & 22 deletions packages/cli/src/lib/webpack/webpack-base-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ function resolveTsconfig(cwd, isProd) {
* @returns {import('webpack').Configuration}
*/
module.exports = function createBaseConfig(env) {
const { cwd, isProd, isWatch, src, source } = env;
const { cwd, isProd, src, source } = env;
const IS_SOURCE_PREACT_X_OR_ABOVE = isInstalledVersionPreactXOrAbove(cwd);
// Apply base-level `env` values
env.dest = resolve(cwd, env.dest || 'build');
Expand Down Expand Up @@ -216,20 +216,15 @@ module.exports = function createBaseConfig(env) {
],
},
{
// User styles
test: /\.(p?css|less|s[ac]ss|styl)$/,
include: [source('components'), source('routes')],
exclude: /\.module\.(p?css|less|s[ac]ss|styl)$/,
use: [
isWatch
? require.resolve('style-loader')
: MiniCssExtractPlugin.loader,
isProd
? MiniCssExtractPlugin.loader
: require.resolve('style-loader'),
{
loader: require.resolve('css-loader'),
options: {
modules: {
localIdentName: '[local]__[hash:base64:5]',
},
importLoaders: 1,
sourceMap: true,
},
},
Expand All @@ -243,18 +238,25 @@ module.exports = function createBaseConfig(env) {
},
},
],
// Don't consider CSS imports dead code even if the
// containing package claims to have no side effects.
// Remove this when webpack adds a warning or an error for this.
// See https://github.com/webpack/webpack/issues/6571
sideEffects: true,
},
{
// External / `node_module` styles
test: /\.(p?css|less|s[ac]ss|styl)$/,
exclude: [source('components'), source('routes')],
test: /\.module\.(p?css|less|s[ac]ss|styl)$/,
use: [
isWatch
? require.resolve('style-loader')
: MiniCssExtractPlugin.loader,
isProd
? MiniCssExtractPlugin.loader
: require.resolve('style-loader'),
{
loader: require.resolve('css-loader'),
options: {
modules: {
localIdentName: '[local]__[hash:base64:5]',
},
importLoaders: 1,
sourceMap: true,
},
},
Expand All @@ -268,11 +270,6 @@ module.exports = function createBaseConfig(env) {
},
},
],
// Don't consider CSS imports dead code even if the
// containing package claims to have no side effects.
// Remove this when webpack adds a warning or an error for this.
// See https://github.com/webpack/webpack/issues/6571
sideEffects: true,
},
{
test: /\.(xml|html|txt|md)$/,
Expand Down Expand Up @@ -362,7 +359,7 @@ module.exports = function createBaseConfig(env) {

mode: isProd ? 'production' : 'development',

devtool: isWatch ? 'eval-cheap-module-source-map' : 'source-map',
devtool: isProd ? 'source-map' : 'eval-cheap-module-source-map',

node: {
__filename: false,
Expand Down
23 changes: 11 additions & 12 deletions packages/cli/tests/build.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,9 @@ describe('preact build', () => {
await rename(join(dir, 'index.js'), join(dir, 'renamed-src/index.js'));
await rename(join(dir, 'style.css'), join(dir, 'renamed-src/style.css'));

await expect(buildFast(dir, { src: 'renamed-src' })).resolves.not.toThrow();
await expect(
buildFast(dir, { src: 'renamed-src' })
).resolves.not.toThrow();
});

it('--dest', async () => {
Expand Down Expand Up @@ -361,18 +363,13 @@ describe('preact build', () => {
expect(builtStylesheet).toMatch('h2{color:green}');
});

it('should use CSS Modules in `routes` and `components` directories', async () => {
let dir = await subject('css-auto-modules');
it('should use plain CSS & CSS Modules together, determining loading method by filename', async () => {
let dir = await subject('css-modules');
await buildFast(dir);
const builtStylesheet = await getOutputFile(dir, /bundle\.\w{5}\.css$/);
const builtSplitStylesheet = await getOutputFile(
dir,
/route-index\.chunk\.\w{5}\.css$/
);

expect(builtStylesheet).toMatch('h1{color:red}');
expect(builtStylesheet).toMatch(/\.text__\w{5}{color:tan}/);
expect(builtSplitStylesheet).toMatch(/\.text__\w{5}{color:red}/);
expect(builtStylesheet).toMatch(/\.text__\w{5}{color:blue}/);
});

it('should inline critical CSS only', async () => {
Expand All @@ -394,12 +391,14 @@ describe('preact build', () => {
expect(builtStylesheet).toMatch('h1{background:#673ab8}');
});

it('should use SASS styles', async () => {
it('should use SASS, SCSS, and CSS Modules for each', async () => {
let dir = await subject('css-sass');
await buildFast(dir);
const builtStylesheet = await getOutputFile(dir, /bundle\.\w{5}\.css$/);

let body = await getBody(dir);
looksLike(body, images.sass);
expect(builtStylesheet).toMatch('h1{background:blue;color:red}');
expect(builtStylesheet).toMatch(/\.text__\w{5}{color:blue}/);
expect(builtStylesheet).toMatch(/\.text__\w{5}{background:red}/);
});
});

Expand Down
50 changes: 20 additions & 30 deletions packages/cli/tests/images/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,18 @@ exports.default = {
'assets/icons/mstile-150x150.png': 9050,
'assets/favicon.ico': 15086,

'ssr-build/ssr-bundle.77c49.css': 1281,
'ssr-build/ssr-bundle.77c49.css.map': 2081,
'ssr-build/ssr-bundle.dba70.css': 1281,
'ssr-build/ssr-bundle.dba70.css.map': 2081,
'ssr-build/ssr-bundle.js': 26049,
'ssr-build/ssr-bundle.js.map': 59971,
'ssr-build/asset-manifest.json': 82,

'bundle.7a791.js': 21563,
'bundle.7a791.js.map': 86009,
'bundle.7a791.legacy.js': 22586,
'bundle.7a791.legacy.js.map': 107151,
'bundle.9bde9.css': 945,
'bundle.9bde9.css.map': 1758,
'bundle.12615.js': 21560,
'bundle.12615.js.map': 85822,
'bundle.12615.legacy.js': 22549,
'bundle.12615.legacy.js.map': 106841,
'bundle.354c3.css': 945,
'bundle.354c3.css.map': 1758,

'dom-polyfills.8a933.legacy.js': 5221,
'dom-polyfills.8a933.legacy.js.map': 18676,
Expand All @@ -31,31 +31,21 @@ exports.default = {
'push-manifest.json': 450,
'asset-manifest.json': 943,

'route-home.chunk.040d2.js': 307,
'route-home.chunk.040d2.js.map': 1684,
'route-home.chunk.040d2.legacy.js': 364,
'route-home.chunk.040d2.legacy.js.map': 1982,
'route-home.chunk.f1c94.css': 112,
'route-home.chunk.f1c94.css.map': 224,
'route-home.chunk.f910e.js': 307,
'route-home.chunk.f910e.js.map': 1516,
'route-home.chunk.f910e.legacy.js': 347,
'route-home.chunk.f910e.legacy.js.map': 1770,
'route-home.chunk.6eaee.css': 112,
'route-home.chunk.6eaee.css.map': 224,

'route-profile.chunk.8c3bd.js': 3121,
'route-profile.chunk.8c3bd.js.map': 12280,
'route-profile.chunk.8c3bd.legacy.js': 3266,
'route-profile.chunk.8c3bd.legacy.js.map': 15684,
'route-profile.chunk.e0d39.css': 118,
'route-profile.chunk.e0d39.css.map': 231,
'route-profile.chunk.ef912.js': 3106,
'route-profile.chunk.ef912.js.map': 12220,
'route-profile.chunk.ef912.legacy.js': 3243,
'route-profile.chunk.ef912.legacy.js.map': 15558,
'route-profile.chunk.0af3e.css': 118,
'route-profile.chunk.0af3e.css.map': 231,
};

exports.sass = `
<body>
<div class="background__sL1ip">
<h1>Header on background</h1>
<p>Paragraph on background</p>
</div>
{{ ... }}
</body>
`;

exports.prerender = {};

exports.prerender.heads = {};
Expand Down
6 changes: 3 additions & 3 deletions packages/cli/tests/images/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ exports.default = [
'src/assets/icons/mstile-150x150.png',
'src/components/app.js',
'src/components/header/index.js',
'src/components/header/style.css',
'src/components/header/style.module.css',
'src/index.js',
'src/manifest.json',
'src/routes/home/index.js',
'src/routes/home/style.css',
'src/routes/home/style.module.css',
'src/routes/profile/index.js',
'src/routes/profile/style.css',
'src/routes/profile/style.module.css',
'src/style/index.css',
'src/sw.js',
'src/template.html',
Expand Down

This file was deleted.

This file was deleted.

4 changes: 0 additions & 4 deletions packages/cli/tests/subjects/css-auto-modules/routes/index.js

This file was deleted.

3 changes: 0 additions & 3 deletions packages/cli/tests/subjects/css-auto-modules/routes/style.css

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import { h } from 'preact';
import Component from './components';
import Route from './routes';
import './global.css';

import './style.css';
import styles from './style.module.css';

export default () => {
return (
<div>
<h1>This is an app with some fancy styles!</h1>
<Route />
<Component />
<h2 class={styles.text}>We can even use CSS Modules!</h2>
</div>
);
};
3 changes: 3 additions & 0 deletions packages/cli/tests/subjects/css-modules/style.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.text {
color: blue;
}
11 changes: 0 additions & 11 deletions packages/cli/tests/subjects/css-sass/components/app/index.js

This file was deleted.

This file was deleted.

17 changes: 15 additions & 2 deletions packages/cli/tests/subjects/css-sass/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,17 @@
import { h } from 'preact';
import App from './components/app';

export default () => <App />;
import './style.sass';
import './style.scss';
import sassStyles from './style.module.sass';
import scssStyles from './style.module.scss';

export default () => {
return (
<div>
<h1>This is an app with some fancy styles!</h1>
<h2 class={`${sassStyles.text} ${scssStyles.text}`}>
We can even use CSS Modules!
</h2>
</div>
);
};
2 changes: 2 additions & 0 deletions packages/cli/tests/subjects/css-sass/style.module.sass
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
.text
color: blue
3 changes: 3 additions & 0 deletions packages/cli/tests/subjects/css-sass/style.module.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.text {
background: red;
}
2 changes: 2 additions & 0 deletions packages/cli/tests/subjects/css-sass/style.sass
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
h1
color: red
3 changes: 3 additions & 0 deletions packages/cli/tests/subjects/css-sass/style.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
h1 {
background: blue;
}

0 comments on commit 79dcc3a

Please sign in to comment.