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

refactor: Determine CSS module by filename, not directory #1714

Merged
merged 5 commits into from
Jul 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
Copy link
Member Author

Choose a reason for hiding this comment

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

Noticed when debugging.

We attempt to log warnings if stats.hasWarnings() is true:

if (field === 'warnings' && stats.hasWarnings()) {
fields = fields.concat(info.warnings.map(addCompilerPrefix));
}

However, info.warnings will always be empty w/out this change.

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;
Copy link
Member Author

Choose a reason for hiding this comment

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

Just a consistency thing. Easier to scan if we're not switching between isProd and isWatch.

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