Skip to content
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
3 changes: 2 additions & 1 deletion src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
warningFactory,
syntaxErrorFactory,
supportTemplateLiteral,
stripBom,
} from "./utils";

export default async function loader(content, map, meta) {
Expand Down Expand Up @@ -174,7 +175,7 @@ export default async function loader(content, map, meta) {
let result;

try {
result = await postcss(plugins).process(content, {
result = await postcss(plugins).process(stripBom(content), {
hideNothingWarning: true,
from: resourcePath,
to: resourcePath,
Expand Down
22 changes: 22 additions & 0 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1471,6 +1471,27 @@ function supportTemplateLiteral(loaderContext) {
return false;
}

const BOM_CODE_POINT = 0xfeff;

// postcss stripped a leading BOM until 8.5.24 and preserves it since, so strip
// it ourselves: concatenated stylesheets must not carry one in the middle.
// `content` is a postcss `Root` when a previous loader handed over its AST,
// and there the BOM lives on the input rather than in the tree.
function stripBom(content) {
if (typeof content === "string") {
return content.charCodeAt(0) === BOM_CODE_POINT
? content.slice(1)
: content;
}

if (content && content.source && content.source.input) {
// eslint-disable-next-line no-param-reassign
content.source.input.hasBOM = false;
}

return content;
}

export {
normalizeOptions,
shouldUseModulesPlugins,
Expand Down Expand Up @@ -1499,4 +1520,5 @@ export {
warningFactory,
syntaxErrorFactory,
supportTemplateLiteral,
stripBom,
};
33 changes: 33 additions & 0 deletions test/__snapshots__/loader.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,39 @@ exports[`loader should not generate console.warn when plugins disabled and hideN

exports[`loader should not generate console.warn when plugins disabled and hideNothingWarning is "true": warnings 1`] = `[]`;

exports[`loader should not keep a BOM in an ast reused from a previous loader: errors 1`] = `[]`;

exports[`loader should not keep a BOM in an ast reused from a previous loader: warnings 1`] = `[]`;

exports[`loader should not pass a BOM added by a previous loader to postcss: errors 1`] = `[]`;

exports[`loader should not pass a BOM added by a previous loader to postcss: module 1`] = `
"// Imports
import ___CSS_LOADER_API_NO_SOURCEMAP_IMPORT___ from "../../../../src/runtime/noSourceMaps.js";
import ___CSS_LOADER_API_IMPORT___ from "../../../../src/runtime/api.js";
var ___CSS_LOADER_EXPORT___ = ___CSS_LOADER_API_IMPORT___(___CSS_LOADER_API_NO_SOURCEMAP_IMPORT___);
// Module
___CSS_LOADER_EXPORT___.push([module.id, \`.first::after {
content: "©";
}
\`, ""]);
// Exports
export default ___CSS_LOADER_EXPORT___;
"
`;

exports[`loader should not pass a BOM added by a previous loader to postcss: result 1`] = `
".first::after {
content: "©";
}
.second::after {
content: "→";
}
"
`;

exports[`loader should not pass a BOM added by a previous loader to postcss: warnings 1`] = `[]`;

exports[`loader should pass queries to other loader: errors 1`] = `[]`;

exports[`loader should pass queries to other loader: module 1`] = `
Expand Down
3 changes: 3 additions & 0 deletions test/fixtures/modules/issue-1678/first.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.first::after {
content: "©";
}
3 changes: 3 additions & 0 deletions test/fixtures/modules/issue-1678/second.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.second::after {
content: "→";
}
9 changes: 9 additions & 0 deletions test/fixtures/modules/issue-1678/source.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import first from "./first.css";
import second from "./second.css";

// Concatenation is where a preserved BOM corrupts: it lands mid-file.
const css = first.toString() + second.toString();

__export__ = css;

export default css;
3 changes: 3 additions & 0 deletions test/fixtures/modules/issue-1678/with-bom-loader.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Emulates sass-loader with `charset: true`, which prefixes a BOM when the
// stylesheet is non-ASCII.
module.exports = (content) => "\uFEFF" + content;
79 changes: 79 additions & 0 deletions test/loader.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import path from "path";

import postcss from "postcss";
import postcssPresetEnv from "postcss-preset-env";

import {
Expand Down Expand Up @@ -207,6 +208,84 @@ describe("loader", () => {
expect(getErrors(stats)).toMatchSnapshot("errors");
});

it("should not pass a BOM added by a previous loader to postcss", async () => {
const BOM = "\uFEFF";
const processSpy = jest.spyOn(Object.getPrototypeOf(postcss()), "process");
const compiler = getCompiler(
"./modules/issue-1678/source.js",
{},
{
module: {
rules: [
{
test: /\.css$/i,
use: [
{ loader: path.resolve(__dirname, "../src") },
{ loader: "./modules/issue-1678/with-bom-loader.js" },
],
},
],
},
},
);

const stats = await compile(compiler);
const executed = getExecutedCode("main.bundle.js", compiler, stats);

expect(processSpy).toHaveBeenCalledTimes(2);

for (const [css] of processSpy.mock.calls) {
expect(css.startsWith(BOM)).toBe(false);
}

// postcss >= 8.5.24 keeps the BOM, so without stripping it lands between
// the two stylesheets and a browser drops the rule after it.
expect(executed).not.toContain(BOM);
expect(executed).toContain(".first::after");
expect(executed).toContain(".second::after");

expect(
getModuleSource("./modules/issue-1678/first.css", stats),
).toMatchSnapshot("module");
expect(executed).toMatchSnapshot("result");
expect(getWarnings(stats)).toMatchSnapshot("warnings");
expect(getErrors(stats)).toMatchSnapshot("errors");

processSpy.mockRestore();
});

it("should not keep a BOM in an ast reused from a previous loader", async () => {
const BOM = "\uFEFF";
const compiler = getCompiler(
"./modules/issue-1678/source.js",
{},
{
module: {
rules: [
{
test: /\.css$/i,
use: [
{ loader: path.resolve(__dirname, "../src") },
{ loader: require.resolve("./helpers/ast-loader") },
{ loader: "./modules/issue-1678/with-bom-loader.js" },
],
},
],
},
},
);

const stats = await compile(compiler);
const executed = getExecutedCode("main.bundle.js", compiler, stats);

expect(executed).not.toContain(BOM);
expect(executed).toContain(".first::after");
expect(executed).toContain(".second::after");

expect(getWarnings(stats)).toMatchSnapshot("warnings");
expect(getErrors(stats)).toMatchSnapshot("errors");
});

it('should work with "sass-loader"', async () => {
const compiler = getCompiler(
"./scss/source.js",
Expand Down
Loading