From b39bc64d67b1a2648d7c86e207f72886b22ad5d9 Mon Sep 17 00:00:00 2001 From: Ben Holloway Date: Mon, 5 Nov 2018 14:29:13 +1100 Subject: [PATCH 1/4] test multiline declaration with unexpected CR characters --- test/cases/orphan-carriage-return.postcss.js | 141 +++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 test/cases/orphan-carriage-return.postcss.js diff --git a/test/cases/orphan-carriage-return.postcss.js b/test/cases/orphan-carriage-return.postcss.js new file mode 100644 index 0000000..a3a00e2 --- /dev/null +++ b/test/cases/orphan-carriage-return.postcss.js @@ -0,0 +1,141 @@ +'use strict'; + +const {join} = require('path'); +const outdent = require('outdent'); +const {test, layer, fs, env, cwd} = require('test-my-cli'); + +const {withCacheBase} = require('../lib/higher-order'); +const {testDefault, testAbsolute, testDebug, testKeepQuery} = require('./common/tests'); +const {buildDevNormal, buildDevNoUrl, buildProdNormal, buildProdNoUrl, buildProdNoDevtool} = require('./common/builds'); +const {assertWebpackOk, assertNoErrors, assertNoMessages} = require('../lib/assert'); + +module.exports = test( + 'orphan-carriage-return', + layer('orphan-carriage-return')( + cwd('.'), + fs({ + 'package.json': withCacheBase('package.json'), + 'webpack.config.js': withCacheBase('webpack.config.js'), + 'node_modules': withCacheBase('node_modules'), + 'src/index.scss': outdent` + .some-class-name { + font-size: calc(${'\r'} + (1px)${'\r'} + ); + background-image: url(data:image/gif;base64,R0lGODlhAQABAIAAAP///wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==); + } + ` + }), + env({ + ENTRY: join('src', 'index.scss') + }), + testDefault( + buildDevNormal( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ), + buildDevNoUrl( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ), + buildProdNormal( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ), + buildProdNoUrl( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ), + buildProdNoDevtool( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ) + ), + testAbsolute( + buildDevNormal( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ), + buildDevNoUrl( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ), + buildProdNormal( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ), + buildProdNoUrl( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ), + buildProdNoDevtool( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ) + ), + testDebug( + buildDevNormal( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ), + buildDevNoUrl( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ), + buildProdNormal( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ), + buildProdNoUrl( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ), + buildProdNoDevtool( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ) + ), + testKeepQuery( + buildDevNormal( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ), + buildDevNoUrl( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ), + buildProdNormal( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ), + buildProdNoUrl( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ), + buildProdNoDevtool( + assertWebpackOk, + assertNoErrors, + assertNoMessages + ) + ) + ) +); From ed9f283eab509209593caeb5d4a2b1f78f603dd6 Mon Sep 17 00:00:00 2001 From: Ben Holloway Date: Sun, 17 Feb 2019 13:09:28 +1100 Subject: [PATCH 2/4] remove libsass orphan CR before postcss as seen in #107 --- packages/resolve-url-loader/README.md | 34 +++-- packages/resolve-url-loader/index.js | 4 +- .../resolve-url-loader/lib/engine/postcss.js | 18 ++- test/cases/common/tests.js | 14 ++ test/cases/orphan-carriage-return.postcss.js | 124 ++++++------------ 5 files changed, 97 insertions(+), 97 deletions(-) diff --git a/packages/resolve-url-loader/README.md b/packages/resolve-url-loader/README.md index 673f68b..ba6b073 100644 --- a/packages/resolve-url-loader/README.md +++ b/packages/resolve-url-loader/README.md @@ -110,16 +110,17 @@ Refer to `test` directory for full webpack configurations (as used in automated ## Options -| option | type | default | | description | -|------------|----------------------------|-------------|----------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -|`engine` | `'rework'`
`'postcss'` | `'postcss'` | | The css parser engine. | -|`sourceMap` | boolean | `false` | | Generate a source-map. | -|`keepQuery` | boolean | `false` | | Keep query-string and/or hash suffixes.
e.g. `url('./MyFont.eot?#iefix')`
Be aware downstream loaders may remove query-string or hash. | -|`debug` | boolean | `false` | | Display debug information. | -|`silent` | boolean | `false` | | Do **not** display warnings. | -|`root` | string | _unset_ | | Similar to the (now defunct) option in `css-loader`.
This string, possibly empty, is prepended to absolute URIs.
Absolute URIs are only processed if this option is set. | -|`join` | function | _inbuilt_ | advanced | Custom join function.
Use custom javascript to fix asset paths on a per-case basis.
Refer to the default implementation for more information. | -|`absolute` | boolean | `false` | useless | Forces URIs to be output as absolute file paths.
This is retained for historical compatibility but is likely to be removed in the future, so let me know if you use it. | +| option | type | default | | description | +|-------------|----------------------------|-------------|----------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `engine` | `'rework'`
`'postcss'` | `'postcss'` | | The css parser engine. | +| `sourceMap` | boolean | `false` | | Generate a source-map. | +| `keepQuery` | boolean | `false` | | Keep query-string and/or hash suffixes.
e.g. `url('./MyFont.eot?#iefix')`
Be aware downstream loaders may remove query-string or hash. | +| `removeCR` | boolean | `false` | | Convert orphan CR to whitespace (postcss only).
See known issues below. | +| `debug` | boolean | `false` | | Display debug information. | +| `silent` | boolean | `false` | | Do **not** display warnings. | +| `root` | string | _unset_ | | Similar to the (now defunct) option in `css-loader`.
This string, possibly empty, is prepended to absolute URIs.
Absolute URIs are only processed if this option is set. | +| `join` | function | _inbuilt_ | advanced | Custom join function.
Use custom javascript to fix asset paths on a per-case basis.
Refer to the default implementation for more information. | +| `absolute` | boolean | `false` | useless | Forces URIs to be output as absolute file paths.
This is retained for historical compatibility but is likely to be removed in the future, so let me know if you use it. | ## How it works @@ -180,6 +181,19 @@ However recall that any paths that _are_ processed will have windows back-slash It can also be useful to process absolute URIs if you have a custom `join` function and want to process all paths. However this is perhaps better done with some separate `postcss` plugin. +## Orphan carriage-return characters + +Under some conditions libsass outputs naked `CR` characters which it does **not** consider newlines. However `postcss` does consider these newlines. The result in a source-map offset which can crash `resolve-url-loader`. + +This is not fully understood but is associated with multiline declarations. + +**Solutions** +* Try the node-sass [linefeed](https://github.com/sass/node-sass#linefeed--v300) option by way of `sass-loader`. + +**Work arounds** +* Enable `removeCR` option [here](#option). +* Remove linebreaks in declarations. + ## Getting help Webpack is difficult to configure but extremely rewarding. diff --git a/packages/resolve-url-loader/index.js b/packages/resolve-url-loader/index.js index 8ba2beb..b4e365e 100644 --- a/packages/resolve-url-loader/index.js +++ b/packages/resolve-url-loader/index.js @@ -50,6 +50,7 @@ function resolveUrlLoader(content, sourceMap) { silent : false, absolute : false, keepQuery: false, + removeCR : false, root : false, debug : false, join : joinFn.defaultJoin @@ -165,7 +166,8 @@ function resolveUrlLoader(content, sourceMap) { outputSourceMap : !!options.sourceMap, transformDeclaration: valueProcessor(loader.resourcePath, options), absSourceMap : absSourceMap, - sourceMapConsumer : sourceMapConsumer + sourceMapConsumer : sourceMapConsumer, + removeCR : options.removeCR })) .catch(onFailure) .then(onSuccess); diff --git a/packages/resolve-url-loader/lib/engine/postcss.js b/packages/resolve-url-loader/lib/engine/postcss.js index d7d7cdd..4243b27 100644 --- a/packages/resolve-url-loader/lib/engine/postcss.js +++ b/packages/resolve-url-loader/lib/engine/postcss.js @@ -4,27 +4,34 @@ */ 'use strict'; -var path = require('path'), +var os = require('os'), + path = require('path'), postcss = require('postcss'); var fileProtocol = require('../file-protocol'); +var ORPHAN_CR_REGEX = /\r(?!\n)(.|\n)?/g; + /** * Process the given CSS content into reworked CSS content. * * @param {string} sourceFile The absolute path of the file being processed * @param {string} sourceContent CSS content without source-map * @param {{outputSourceMap: boolean, transformDeclaration:function, absSourceMap:object, - * sourceMapConsumer:object}} params Named parameters + * sourceMapConsumer:object, removeCR:boolean}} params Named parameters * @return {{content: string, map: object}} Reworked CSS and optional source-map */ function process(sourceFile, sourceContent, params) { + // #107 libsass emits orphan CR not considered newline, postcss does consider newline (content vs source-map mismatch) + var correctedContent = params.removeCR && (os.EOL !== '\r') ? + sourceContent.replace(ORPHAN_CR_REGEX, ' $1') : + sourceContent; // prepend file protocol to all sources to avoid problems with source map return postcss([ postcss.plugin('postcss-resolve-url', postcssPlugin) ]) - .process(sourceContent, { + .process(correctedContent, { from: fileProtocol.prepend(sourceFile), map : params.outputSourceMap && { prev : !!params.absSourceMap && fileProtocol.prepend(params.absSourceMap), @@ -69,7 +76,10 @@ function process(sourceFile, sourceContent, params) { } // source-map present but invalid entry else if (params.sourceMapConsumer) { - throw new Error('source-map information is not available at url() declaration'); + throw new Error( + 'source-map information is not available at url() declaration ' + + (ORPHAN_CR_REGEX.test(sourceContent) ? '(found orphan CR, try removeCR option)' : '(no orphan CR found)') + ); } } } diff --git a/test/cases/common/tests.js b/test/cases/common/tests.js index 025320c..79a76cc 100644 --- a/test/cases/common/tests.js +++ b/test/cases/common/tests.js @@ -101,6 +101,20 @@ exports.testKeepQuery = (...rest) => ) ); +exports.testRemoveCR = (...rest) => + test( + 'removeCR=true', + layer()( + env({ + LOADER_QUERY: 'removeCR', + LOADER_OPTIONS: {removeCR: true}, + OUTPUT: 'remove-CR' + }), + ...rest, + test('validate', assertStderr('options.removeCR')(1)`removeCR: true`) + ) + ); + exports.testRoot = (...rest) => test( 'root=empty', diff --git a/test/cases/orphan-carriage-return.postcss.js b/test/cases/orphan-carriage-return.postcss.js index a3a00e2..78d0281 100644 --- a/test/cases/orphan-carriage-return.postcss.js +++ b/test/cases/orphan-carriage-return.postcss.js @@ -5,9 +5,22 @@ const outdent = require('outdent'); const {test, layer, fs, env, cwd} = require('test-my-cli'); const {withCacheBase} = require('../lib/higher-order'); -const {testDefault, testAbsolute, testDebug, testKeepQuery} = require('./common/tests'); -const {buildDevNormal, buildDevNoUrl, buildProdNormal, buildProdNoUrl, buildProdNoDevtool} = require('./common/builds'); -const {assertWebpackOk, assertNoErrors, assertNoMessages} = require('../lib/assert'); +const {testDefault, testRemoveCR} = require('./common/tests'); +const { + buildDevNormal, buildDevBail, buildDevNoUrl, buildProdNormal, buildProdBail, buildProdNoUrl, buildProdNoDevtool +} = require('./common/builds'); +const { + onlyMeta, assertWebpackOk, assertWebpackNotOk, assertStdout, assertNoErrors, assertNoMessages +} = require('../lib/assert'); + +// Allow 1-4 errors +// - known-issue in extract-text-plugin, failed loaders will rerun webpack>=2 +// - webpack may repeat errors with a header line taken from the parent loader +const assertCssError = assertStdout('error')([1, 4])` + ^[ ]*ERROR[^\n]* + ([^\n]+\n){0,2}[^\n]*resolve-url-loader:[ ]*CSS error + [ ]+source-map information is not available at url\(\) declaration \(found orphan CR, try removeCR option\) + `; module.exports = test( 'orphan-carriage-return', @@ -30,87 +43,34 @@ module.exports = test( ENTRY: join('src', 'index.scss') }), testDefault( - buildDevNormal( - assertWebpackOk, - assertNoErrors, - assertNoMessages - ), - buildDevNoUrl( - assertWebpackOk, - assertNoErrors, - assertNoMessages - ), - buildProdNormal( - assertWebpackOk, - assertNoErrors, - assertNoMessages - ), - buildProdNoUrl( - assertWebpackOk, - assertNoErrors, - assertNoMessages - ), - buildProdNoDevtool( - assertWebpackOk, - assertNoErrors, - assertNoMessages - ) - ), - testAbsolute( - buildDevNormal( - assertWebpackOk, - assertNoErrors, - assertNoMessages - ), - buildDevNoUrl( - assertWebpackOk, - assertNoErrors, - assertNoMessages - ), - buildProdNormal( - assertWebpackOk, - assertNoErrors, - assertNoMessages - ), - buildProdNoUrl( - assertWebpackOk, - assertNoErrors, - assertNoMessages - ), - buildProdNoDevtool( - assertWebpackOk, - assertNoErrors, - assertNoMessages - ) - ), - testDebug( - buildDevNormal( - assertWebpackOk, - assertNoErrors, - assertNoMessages - ), - buildDevNoUrl( - assertWebpackOk, - assertNoErrors, - assertNoMessages - ), - buildProdNormal( - assertWebpackOk, - assertNoErrors, - assertNoMessages - ), - buildProdNoUrl( - assertWebpackOk, - assertNoErrors, - assertNoMessages - ), - buildProdNoDevtool( - assertWebpackOk, - assertNoErrors, - assertNoMessages + onlyMeta('meta.version.webpack == 1')( + buildDevBail( + assertWebpackNotOk + ), + buildDevNormal( + assertWebpackOk, + assertCssError + ), + buildProdBail( + assertWebpackNotOk + ), + buildProdNormal( + assertWebpackOk, + assertCssError + ) + ), + onlyMeta('meta.version.webpack > 1')( + buildDevNormal( + assertWebpackNotOk, + assertCssError + ), + buildProdNormal( + assertWebpackNotOk, + assertCssError + ) ) ), - testKeepQuery( + testRemoveCR( buildDevNormal( assertWebpackOk, assertNoErrors, From 87d409174ff69af16cc83c2d28edfddaa670dcaf Mon Sep 17 00:00:00 2001 From: Ben Holloway Date: Wed, 3 Apr 2019 20:12:53 +1100 Subject: [PATCH 3/4] better explain orphan CR in readme --- packages/resolve-url-loader/README.md | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/packages/resolve-url-loader/README.md b/packages/resolve-url-loader/README.md index ba6b073..1449a8d 100644 --- a/packages/resolve-url-loader/README.md +++ b/packages/resolve-url-loader/README.md @@ -161,6 +161,8 @@ All `webpack1`-`webpack4` with contemporaneous loaders/plugins. Refer to `test` directory for full webpack configurations (as used in automated tests). +Some edge cases with `libsass` on `Windows` (see below). + ### Engines The `engine:postcss` is by far the more reliable option. @@ -181,11 +183,22 @@ However recall that any paths that _are_ processed will have windows back-slash It can also be useful to process absolute URIs if you have a custom `join` function and want to process all paths. However this is perhaps better done with some separate `postcss` plugin. -## Orphan carriage-return characters +### Windows line breaks + +Normal windows linebreaks are `CRLF`. But sometimes libsass will output single `CR` characters. -Under some conditions libsass outputs naked `CR` characters which it does **not** consider newlines. However `postcss` does consider these newlines. The result in a source-map offset which can crash `resolve-url-loader`. +This problem is specific to multiline declarations. Refer to the [libsass bug #2693](https://github.com/sass/libsass/issues/2693). -This is not fully understood but is associated with multiline declarations. +If you have _any_ such multiline declarations preceding `url()` statements it will fail your build. + +Libsass doesn't consider these orphan `CR` to be newlines but `postcss` engine does. The result being an offset in source-map line-numbers which crashes `resolve-url-loader`. + +``` +Module build failed: Error: resolve-url-loader: CSS error + source-map information is not available at url() declaration +``` + +Some users find the node-sass `linefeed` option solves the problem. **Solutions** * Try the node-sass [linefeed](https://github.com/sass/node-sass#linefeed--v300) option by way of `sass-loader`. @@ -194,6 +207,11 @@ This is not fully understood but is associated with multiline declarations. * Enable `removeCR` option [here](#option). * Remove linebreaks in declarations. +**Diagnosis** +1. Run a stand-alone sass build `npx node-sass index.scss output.css` +2. Use a hex editor to check line endings `Format-Hex output.css` +3. Expect `0DOA` (or desired) line endings. Single `0D` confirms this problem. + ## Getting help Webpack is difficult to configure but extremely rewarding. From 85e6d8ae27675fd7dc2c621598d64561f2b9b75b Mon Sep 17 00:00:00 2001 From: Ben Holloway Date: Thu, 4 Apr 2019 08:54:20 +1100 Subject: [PATCH 4/4] bump the beta --- packages/resolve-url-loader/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/resolve-url-loader/package.json b/packages/resolve-url-loader/package.json index 5ab2ac3..f91ebc5 100644 --- a/packages/resolve-url-loader/package.json +++ b/packages/resolve-url-loader/package.json @@ -1,6 +1,6 @@ { "name": "resolve-url-loader", - "version": "3.0.1", + "version": "3.1.0-beta.2", "description": "Webpack loader that resolves relative paths in url() statements based on the original source file", "main": "index.js", "repository": {