diff --git a/src/index.js b/src/index.js index 3c89c371..c9ff57cb 100644 --- a/src/index.js +++ b/src/index.js @@ -28,6 +28,7 @@ import { warningFactory, syntaxErrorFactory, supportTemplateLiteral, + stripBom, } from "./utils"; export default async function loader(content, map, meta) { @@ -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, diff --git a/src/utils.js b/src/utils.js index 22c74e0b..87c40a43 100644 --- a/src/utils.js +++ b/src/utils.js @@ -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, @@ -1499,4 +1520,5 @@ export { warningFactory, syntaxErrorFactory, supportTemplateLiteral, + stripBom, }; diff --git a/test/__snapshots__/loader.test.js.snap b/test/__snapshots__/loader.test.js.snap index 7d01bc9a..102b96ad 100644 --- a/test/__snapshots__/loader.test.js.snap +++ b/test/__snapshots__/loader.test.js.snap @@ -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`] = ` diff --git a/test/fixtures/modules/issue-1678/first.css b/test/fixtures/modules/issue-1678/first.css new file mode 100644 index 00000000..79e0c7cf --- /dev/null +++ b/test/fixtures/modules/issue-1678/first.css @@ -0,0 +1,3 @@ +.first::after { + content: "©"; +} diff --git a/test/fixtures/modules/issue-1678/second.css b/test/fixtures/modules/issue-1678/second.css new file mode 100644 index 00000000..a7dff145 --- /dev/null +++ b/test/fixtures/modules/issue-1678/second.css @@ -0,0 +1,3 @@ +.second::after { + content: "→"; +} diff --git a/test/fixtures/modules/issue-1678/source.js b/test/fixtures/modules/issue-1678/source.js new file mode 100644 index 00000000..14c8c9df --- /dev/null +++ b/test/fixtures/modules/issue-1678/source.js @@ -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; diff --git a/test/fixtures/modules/issue-1678/with-bom-loader.js b/test/fixtures/modules/issue-1678/with-bom-loader.js new file mode 100644 index 00000000..06aa1e81 --- /dev/null +++ b/test/fixtures/modules/issue-1678/with-bom-loader.js @@ -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; diff --git a/test/loader.test.js b/test/loader.test.js index 916cbddf..5c2771d0 100644 --- a/test/loader.test.js +++ b/test/loader.test.js @@ -1,5 +1,6 @@ import path from "path"; +import postcss from "postcss"; import postcssPresetEnv from "postcss-preset-env"; import { @@ -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",