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 Aug 1, 2022
1 parent cc65fcb commit 7c31f6a
Show file tree
Hide file tree
Showing 23 changed files with 123 additions and 117 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.
26 changes: 18 additions & 8 deletions packages/cli/global.d.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,21 @@
declare global {
const __webpack_public_path__: string;
namespace jest {
interface Matchers<R> {
toBeCloseInSize(receivedSize: number, expectedSize: number): R;
toFindMatchingKey(receivedKey: string): R;
}
declare const __webpack_public_path__: string;

declare namespace jest {
interface Matchers<R> {
toBeCloseInSize(receivedSize: number, expectedSize: number): R;
toFindMatchingKey(receivedKey: string): R;
}
}

export {};
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;
}
14 changes: 8 additions & 6 deletions packages/cli/lib/commands/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ const { addScripts, install, initGit } = require('../lib/setup');
const { validateArgs } = require('./validate-args');

const ORG = 'preactjs-templates';
const RGX = /\.(woff2?|ttf|eot|jpe?g|ico|png|gif|webp|mp4|mov|ogg|webm)(\?.*)?$/i;
const RGX =
/\.(woff2?|ttf|eot|jpe?g|ico|png|gif|webp|mp4|mov|ogg|webm)(\?.*)?$/i;
const isMedia = str => RGX.test(str);
const capitalize = str => str.charAt(0).toUpperCase() + str.substring(1);

Expand Down Expand Up @@ -176,11 +177,7 @@ async function fetchTemplates() {
// If cache file doesn't exist, then hit the API and fetch the data
if (!existsSync(cacheFilePath)) {
const repos = await fetch(TEMPLATES_REPO_URL).then(r => r.json());
await writeFile(
cacheFilePath,
JSON.stringify(repos, null, 2),
'utf-8'
);
await writeFile(cacheFilePath, JSON.stringify(repos, null, 2), 'utf-8');
}

// update the cache file without blocking the rest of the tasks.
Expand Down Expand Up @@ -268,6 +265,11 @@ async function command(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 += '#main';
}
}

if (!existsSync(resolve(cwd, dest, 'src'))) {
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/lib/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/lib/lib/webpack/webpack-base-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ function getSassConfiguration(...includePaths) {
* @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 @@ -219,20 +219,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 @@ -246,18 +241,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 @@ -271,11 +273,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 @@ -366,7 +363,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 @@ -184,7 +184,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 @@ -369,18 +371,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 @@ -402,12 +399,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': 2070,
'ssr-build/ssr-bundle.c9123.css': 1281,
'ssr-build/ssr-bundle.c9123.css.map': 2070,
'ssr-build/ssr-bundle.js': 9801,
'ssr-build/ssr-bundle.js.map': 30625,
'ssr-build/asset-manifest.json': 82,

'bundle.6d2a5.js': 21323,
'bundle.6d2a5.js.map': 85534,
'bundle.6d2a5.legacy.js': 22514,
'bundle.6d2a5.legacy.js.map': 106422,
'bundle.9bde9.css': 945,
'bundle.9bde9.css.map': 1758,
'bundle.3f496.js': 21323,
'bundle.3f496.js.map': 85534,
'bundle.3f496.legacy.js': 22514,
'bundle.3f496.legacy.js.map': 106422,
'bundle.a2557.css': 945,
'bundle.a2557.css.map': 1758,

'dom-polyfills.c88f4.legacy.js': 5252,
'dom-polyfills.c88f4.legacy.js.map': 18836,
Expand All @@ -31,31 +31,21 @@ exports.default = {
'push-manifest.json': 450,
'asset-manifest.json': 943,

'route-home.chunk.25907.js': 306,
'route-home.chunk.25907.js.map': 1615,
'route-home.chunk.25907.legacy.js': 363,
'route-home.chunk.25907.legacy.js.map': 1913,
'route-home.chunk.f1c94.css': 112,
'route-home.chunk.f1c94.css.map': 224,
'route-home.chunk.b9f12.js': 306,
'route-home.chunk.b9f12.js.map': 1615,
'route-home.chunk.b9f12.legacy.js': 363,
'route-home.chunk.b9f12.legacy.js.map': 1913,
'route-home.chunk.6eaee.css': 112,
'route-home.chunk.6eaee.css.map': 224,

'route-profile.chunk.d1e1a.js': 2469,
'route-profile.chunk.d1e1a.js.map': 9736,
'route-profile.chunk.d1e1a.legacy.js': 2624,
'route-profile.chunk.d1e1a.legacy.js.map': 12373,
'route-profile.chunk.e0d39.css': 118,
'route-profile.chunk.e0d39.css.map': 231,
'route-profile.chunk.d94b1.js': 2469,
'route-profile.chunk.d94b1.js.map': 9736,
'route-profile.chunk.d94b1.legacy.js': 2624,
'route-profile.chunk.d94b1.legacy.js.map': 12373,
'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
Loading

0 comments on commit 7c31f6a

Please sign in to comment.