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.

Sign up for GitHub

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

fix: lightningcss-loader not work when using PostCSS #3396

Merged
merged 4 commits into from
Sep 6, 2024
Merged

fix: lightningcss-loader not work when using PostCSS #3396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4b31457
fix: lightningcss-loader not work when using PostCSS
chenjiahan Sep 6, 2024
10033fe
fix
chenjiahan Sep 6, 2024
6b9c21f
fix
chenjiahan Sep 6, 2024
c4e78ce
fix: snapshot
chenjiahan Sep 6, 2024
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
27 changes: 27 additions & 0 deletions e2e/cases/css/style-loader-lightningcss/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { build, rspackOnlyTest } from '@e2e/helper';
import { expect } from '@playwright/test';

const fixtures = __dirname;

rspackOnlyTest(
'should use lightningcss-loader to transform and minify CSS when injectStyles is true',
async () => {
const rsbuild = await build({
cwd: fixtures,
});

// injectStyles worked
const files = await rsbuild.unwrapOutputJSON();

// should inline minified css
const indexJsFile = Object.keys(files).find(
(file) => file.includes('index.') && file.endsWith('.js'),
)!;

expect(
files[indexJsFile].includes(
'@media (-webkit-min-device-pixel-ratio:2),(min-resolution:2dppx){.item{-webkit-user-select:none;user-select:none;background:linear-gradient(#fff,#000);transition:all .5s}}',
),
).toBeTruthy();
},
);
7 changes: 7 additions & 0 deletions e2e/cases/css/style-loader-lightningcss/rsbuild.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { defineConfig } from '@rsbuild/core';

export default defineConfig({
output: {
injectStyles: true,
},
});
7 changes: 7 additions & 0 deletions e2e/cases/css/style-loader-lightningcss/src/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
@media (min-resolution: 2dppx) {
.item {
transition: all 0.5s;
user-select: none;
background: linear-gradient(to bottom, white, black);
}
}
1 change: 1 addition & 0 deletions e2e/cases/css/style-loader-lightningcss/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './index.css';
2 changes: 2 additions & 0 deletions e2e/cases/css/tailwindcss-vendor-prefix/.browserslistrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
IE >= 11
Chrome >= 10
20 changes: 20 additions & 0 deletions e2e/cases/css/tailwindcss-vendor-prefix/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { build, rspackOnlyTest } from '@e2e/helper';
import { expect } from '@playwright/test';

rspackOnlyTest(
'should generate tailwindcss utilities with vendor prefixes correctly',
async () => {
const rsbuild = await build({
cwd: __dirname,
});

const files = await rsbuild.unwrapOutputJSON();
const indexCssFile = Object.keys(files).find(
(file) => file.includes('index.') && file.endsWith('.css'),
)!;

expect(files[indexCssFile]).toContain('-webkit-user-select: none;');
expect(files[indexCssFile]).toContain('-ms-user-select: none;');
expect(files[indexCssFile]).toContain('user-select: none;');
},
);
9 changes: 9 additions & 0 deletions e2e/cases/css/tailwindcss-vendor-prefix/postcss.config.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
const path = require('node:path');

module.exports = {
plugins: {
tailwindcss: {
config: path.join(__dirname, './tailwind.config.cjs'),
},
},
};
10 changes: 10 additions & 0 deletions e2e/cases/css/tailwindcss-vendor-prefix/rsbuild.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { defineConfig } from '@rsbuild/core';

export default defineConfig({
html: {
template: './src/index.html',
},
output: {
minify: false,
},
});
1 change: 1 addition & 0 deletions e2e/cases/css/tailwindcss-vendor-prefix/src/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@tailwind utilities;
7 changes: 7 additions & 0 deletions e2e/cases/css/tailwindcss-vendor-prefix/src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<!doctype html>
<html>
<head></head>
<body>
<h1 class="select-none">Hello world!</h1>
</body>
</html>
1 change: 1 addition & 0 deletions e2e/cases/css/tailwindcss-vendor-prefix/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './index.css';
10 changes: 10 additions & 0 deletions e2e/cases/css/tailwindcss-vendor-prefix/tailwind.config.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
const path = require('node:path');

/** @type {import('tailwindcss').Config} */
module.exports = {
content: [path.join(__dirname, './src/**/*.{html,js,ts,jsx,tsx}')],
theme: {
extend: {},
},
plugins: [],
};
4 changes: 4 additions & 0 deletions packages/compat/webpack/tests/__snapshots__/default.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ exports[`applyDefaultPlugins > should apply default plugins correctly 1`] = `
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 0,
"modules": {
"auto": true,
Expand Down Expand Up @@ -418,6 +419,7 @@ exports[`applyDefaultPlugins > should apply default plugins correctly when produ
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 0,
"modules": {
"auto": true,
Expand Down Expand Up @@ -776,6 +778,7 @@ exports[`applyDefaultPlugins > should apply default plugins correctly when targe
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 0,
"modules": {
"auto": true,
Expand Down Expand Up @@ -1081,6 +1084,7 @@ exports[`applyDefaultPlugins > should apply default plugins correctly when targe
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 0,
"modules": {
"auto": true,
Expand Down
22 changes: 15 additions & 7 deletions packages/core/prebundle.config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,6 @@ import fs from 'node:fs';
*/
import { join } from 'node:path';

// The package size of `schema-utils` is large, and validate has a performance overhead of tens of ms.
// So we skip the validation and let TypeScript to ensure type safety.
const writeEmptySchemaUtils = (task) => {
const schemaUtilsPath = join(task.distPath, 'schema-utils.js');
fs.writeFileSync(schemaUtilsPath, 'module.exports.validate = () => {};');
};

// postcss-loader and css-loader use `semver` to compare PostCSS ast version,
// Rsbuild uses the same PostCSS version and do not need the comparison.
const writeEmptySemver = (task) => {
Expand Down Expand Up @@ -168,6 +161,21 @@ export default {
postcss: '../postcss',
picocolors: '../picocolors',
},
beforeBundle(task) {
// Temp fix for https://github.com/web-infra-dev/rspack/issues/7819
replaceFileContent(join(task.depPath, 'dist/index.js'), (content) =>
content.replaceAll(
'if (meta) {',
'if (meta && !rawOptions._skipReuseAST) {',
),
);
replaceFileContent(join(task.depPath, 'dist/index.js'), (content) =>
content.replaceAll(
'this.getOptions(_options.default)',
'this.getOptions()',
),
);
},
afterBundle: writeEmptySemver,
},
{
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/plugins/css.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ const getCSSLoaderOptions = ({
localIdentName,
},
sourceMap: config.output.sourceMap.css,
_skipReuseAST: config.tools.lightningcssLoader !== false,
};

const mergedCssLoaderOptions = reduceConfigs({
Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/types/thirdParty.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,11 @@ export interface CSSLoaderOptions {
* @default 'array'
*/
exportType?: 'array' | 'string' | 'css-style-sheet';
/**
* Temp fix for https://github.com/web-infra-dev/rspack/issues/7819
* @private
*/
_skipReuseAST?: boolean;
}

export type StyleLoaderInjectType =
Expand Down
1 change: 1 addition & 0 deletions packages/core/tests/__snapshots__/builder.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ exports[`should use rspack as default bundler > apply rspack correctly 1`] = `
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 1,
"modules": {
"auto": true,
Expand Down
5 changes: 5 additions & 0 deletions packages/core/tests/__snapshots__/css.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ exports[`plugin-css > should use custom cssModules rule when using output.cssMod
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 1,
"modules": {
"auto": [Function],
Expand Down Expand Up @@ -68,6 +69,7 @@ exports[`plugin-css injectStyles > should apply ignoreCssLoader when injectStyle
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 0,
"modules": {
"auto": true,
Expand Down Expand Up @@ -108,6 +110,7 @@ exports[`plugin-css injectStyles > should use css-loader + style-loader when inj
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 1,
"modules": {
"auto": true,
Expand Down Expand Up @@ -156,6 +159,7 @@ exports[`should ensure isolation of PostCSS config objects between different bui
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 2,
"modules": {
"auto": true,
Expand Down Expand Up @@ -214,6 +218,7 @@ exports[`should ensure isolation of PostCSS config objects between different bui
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 2,
"modules": {
"auto": true,
Expand Down
4 changes: 4 additions & 0 deletions packages/core/tests/__snapshots__/default.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ exports[`applyDefaultPlugins > should apply default plugins correctly 1`] = `
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 1,
"modules": {
"auto": true,
Expand Down Expand Up @@ -437,6 +438,7 @@ exports[`applyDefaultPlugins > should apply default plugins correctly when prod
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 1,
"modules": {
"auto": true,
Expand Down Expand Up @@ -893,6 +895,7 @@ exports[`applyDefaultPlugins > should apply default plugins correctly when targe
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 0,
"modules": {
"auto": true,
Expand Down Expand Up @@ -1227,6 +1230,7 @@ exports[`tools.rspack > should match snapshot 1`] = `
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 1,
"modules": {
"auto": true,
Expand Down
2 changes: 2 additions & 0 deletions packages/core/tests/__snapshots__/environments.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1429,6 +1429,7 @@ exports[`environment config > tools.rspack / bundlerChain can be used in environ
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 1,
"modules": {
"auto": true,
Expand Down Expand Up @@ -1770,6 +1771,7 @@ exports[`environment config > tools.rspack / bundlerChain can be used in environ
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 0,
"modules": {
"auto": true,
Expand Down
4 changes: 4 additions & 0 deletions packages/plugin-less/tests/__snapshots__/index.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ exports[`plugin-less > should add less-loader 1`] = `
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 2,
"modules": {
"auto": true,
Expand Down Expand Up @@ -70,6 +71,7 @@ exports[`plugin-less > should add less-loader and css-loader when injectStyles 1
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 2,
"modules": {
"auto": true,
Expand Down Expand Up @@ -128,6 +130,7 @@ exports[`plugin-less > should add less-loader with excludes 1`] = `
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 2,
"modules": {
"auto": true,
Expand Down Expand Up @@ -183,6 +186,7 @@ exports[`plugin-less > should add less-loader with tools.less 1`] = `
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 2,
"modules": {
"auto": true,
Expand Down
1 change: 1 addition & 0 deletions packages/plugin-react/tests/__snapshots__/index.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ exports[`plugins/react > should work with swc-loader 1`] = `
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 1,
"modules": {
"auto": true,
Expand Down
3 changes: 3 additions & 0 deletions packages/plugin-sass/tests/__snapshots__/index.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ exports[`plugin-sass > should add sass-loader 1`] = `
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 3,
"modules": {
"auto": true,
Expand Down Expand Up @@ -72,6 +73,7 @@ exports[`plugin-sass > should add sass-loader and css-loader when injectStyles 1
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 3,
"modules": {
"auto": true,
Expand Down Expand Up @@ -132,6 +134,7 @@ exports[`plugin-sass > should add sass-loader with excludes 1`] = `
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 3,
"modules": {
"auto": true,
Expand Down
2 changes: 2 additions & 0 deletions packages/plugin-stylus/tests/__snapshots__/index.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ exports[`plugin-stylus > should add stylus loader config correctly 1`] = `
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 2,
"modules": {
"auto": true,
Expand Down Expand Up @@ -63,6 +64,7 @@ exports[`plugin-stylus > should allow to configure stylus options 1`] = `
{
"loader": "<ROOT>/packages/core/compiled/css-loader/index.js",
"options": {
"_skipReuseAST": true,
"importLoaders": 2,
"modules": {
"auto": true,
Expand Down
Loading