Skip to content

Commit

Permalink
fix: emit warnings on broken @import (#806)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: invalid `@import` at rules now emit warnings
  • Loading branch information
evilebottnawi authored Nov 27, 2018
1 parent 8f0232b commit 4bdf08b
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 10 deletions.
20 changes: 20 additions & 0 deletions lib/Warning.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
module.exports = class Warning extends Error {
constructor(warning) {
super(warning);
const { text, line, column } = warning;
this.name = 'Warning';

// Based on https://github.com/postcss/postcss/blob/master/lib/warning.es6#L74
// We don't need `plugin` properties.
this.message = `${this.name}\n\n`;

if (typeof line !== 'undefined') {
this.message += `(${line}:${column}) `;
}

this.message += `${text}`;

// We don't need stack https://github.com/postcss/postcss/blob/master/docs/guidelines/runner.md#31-dont-show-js-stack-for-csssyntaxerror
this.stack = false;
}
};
4 changes: 4 additions & 0 deletions lib/postcss-css-loader-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ module.exports = postcss.plugin(
}

if (!url.replace(/\s/g, '').length) {
result.warn(`Unable to find uri in '${atrule.toString()}'`, {
node: atrule,
});

return;
}

Expand Down
7 changes: 7 additions & 0 deletions lib/processCss.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const modulesValues = require('postcss-modules-values');

const cssLoaderParser = require('./postcss-css-loader-parser');

const Warning = require('./Warning');
const CssSyntaxError = require('./CssSyntaxError');
const { getLocalIdent } = require('./utils');

Expand Down Expand Up @@ -83,6 +84,12 @@ module.exports = function processCss(inputSource, inputMap, options, callback) {
: null,
})
.then((result) => {
result
.warnings()
.forEach((warning) =>
options.loaderContext.emitWarning(new Warning(warning))
);

callback(null, {
source: result.css,
map: result.map && result.map.toJSON(),
Expand Down
62 changes: 61 additions & 1 deletion test/__snapshots__/import-option.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -240,4 +240,64 @@ exports.push([module.id, \\"@import URL(test.css);\\\\n@import url();\\\\n@impor
"
`;

exports[`import option true: warnings 1`] = `Array []`;
exports[`import option true: warnings 1`] = `
Array [
"ModuleWarning: Module Warning (from \`replaced original path\`):
Warning
(12:1) Unable to find uri in '@import url()'",
"ModuleWarning: Module Warning (from \`replaced original path\`):
Warning
(13:1) Unable to find uri in '@import url('')'",
"ModuleWarning: Module Warning (from \`replaced original path\`):
Warning
(14:1) Unable to find uri in '@import url(\\"\\")'",
"ModuleWarning: Module Warning (from \`replaced original path\`):
Warning
(17:1) Unable to find uri in '@import '''",
"ModuleWarning: Module Warning (from \`replaced original path\`):
Warning
(18:1) Unable to find uri in '@import \\"\\"'",
"ModuleWarning: Module Warning (from \`replaced original path\`):
Warning
(19:1) Unable to find uri in '@import \\" \\"'",
"ModuleWarning: Module Warning (from \`replaced original path\`):
Warning
(20:1) Unable to find uri in '@import \\"
\\"'",
"ModuleWarning: Module Warning (from \`replaced original path\`):
Warning
(22:1) Unable to find uri in '@import url()'",
"ModuleWarning: Module Warning (from \`replaced original path\`):
Warning
(23:1) Unable to find uri in '@import url('')'",
"ModuleWarning: Module Warning (from \`replaced original path\`):
Warning
(24:1) Unable to find uri in '@import url(\\"\\")'",
"ModuleWarning: Module Warning (from \`replaced original path\`):
Warning
(37:1) Unable to find uri in '@import '",
"ModuleWarning: Module Warning (from \`replaced original path\`):
Warning
(38:1) Unable to find uri in '@import foo-bar'",
"ModuleWarning: Module Warning (from \`replaced original path\`):
Warning
(40:1) It looks like you didn't end your @import statement correctly. Child nodes are attached to it.",
"ModuleWarning: Module Warning (from \`replaced original path\`):
Warning
(5:1) Unable to find uri in '@import URL(test.css)'",
]
`;
4 changes: 2 additions & 2 deletions test/__snapshots__/loader.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ exports[`loader should compile with empty css entry point: warnings 1`] = `Array

exports[`loader should throw error on invalid css syntax: errors 1`] = `
Array [
[ModuleBuildError: Module build failed (from \`replaced original path\`):
"ModuleBuildError: Module build failed (from \`replaced original path\`):
CssSyntaxError
(2:3) Unknown word
Expand All @@ -402,7 +402,7 @@ CssSyntaxError
| ^
3 | }
4 |
],
",
]
`;

Expand Down
7 changes: 3 additions & 4 deletions test/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,12 +308,11 @@ exports.evaluated = evaluated;

function normalizeErrors(errors) {
return errors.map((error) => {
// eslint-disable-next-line no-param-reassign
error.message = stripAnsi(error.message)
const message = error.toString();

return stripAnsi(message)
.replace(/\(from .*?\)/, '(from `replaced original path`)')
.replace(/at(.*?)\(.*?\)/g, 'at$1(`replaced original path`)');

return error;
});
}

Expand Down
8 changes: 5 additions & 3 deletions test/import-option.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { webpack, evaluated } = require('./helpers');
const { webpack, evaluated, normalizeErrors } = require('./helpers');

describe('import option', () => {
it('true', async () => {
Expand All @@ -11,8 +11,10 @@ describe('import option', () => {
expect(evaluated(module.source, modules)).toMatchSnapshot(
'module (evaluated)'
);
expect(stats.compilation.warnings).toMatchSnapshot('warnings');
expect(stats.compilation.errors).toMatchSnapshot('errors');
expect(normalizeErrors(stats.compilation.warnings)).toMatchSnapshot(
'warnings'
);
expect(normalizeErrors(stats.compilation.errors)).toMatchSnapshot('errors');
});

it('false', async () => {
Expand Down

0 comments on commit 4bdf08b

Please sign in to comment.