From d779eb1982b9feb27c37cda0f7874df02ab5e507 Mon Sep 17 00:00:00 2001 From: Alexey Lavinsky Date: Thu, 24 Sep 2020 14:53:46 +0300 Subject: [PATCH] feat: escape getLocalIdent by default (#1196) BREAKING CHANGE: returned value from the `getLocalIdent` escapes by default, the `exportName` value is always unescaped --- src/utils.js | 65 +++++++++++-------- .../__snapshots__/modules-option.test.js.snap | 58 ++++++++++++++++- test/fixtures/modules/issue-966/issue-966.css | 19 ++++++ test/fixtures/modules/issue-966/issue-966.js | 5 ++ test/modules-option.test.js | 39 +++++++++++ 5 files changed, 157 insertions(+), 29 deletions(-) create mode 100644 test/fixtures/modules/issue-966/issue-966.css create mode 100644 test/fixtures/modules/issue-966/issue-966.js diff --git a/src/utils.js b/src/utils.js index 218c0b73..dd40967a 100644 --- a/src/utils.js +++ b/src/utils.js @@ -49,6 +49,18 @@ const filenameReservedRegex = /[<>:"/\\|?*]/g; // eslint-disable-next-line no-control-regex const reControlChars = /[\u0000-\u001f\u0080-\u009f]/g; +function escapeLocalident(localident) { + return cssesc( + localident + // For `[hash]` placeholder + .replace(/^((-?[0-9])|--)/, '_$1') + .replace(filenameReservedRegex, '-') + .replace(reControlChars, '-') + .replace(/\./g, '-'), + { isIdentifier: true } + ); +} + function defaultGetLocalIdent( loaderContext, localIdentName, @@ -60,19 +72,9 @@ function defaultGetLocalIdent( const request = normalizePath(path.relative(context, resourcePath)); // eslint-disable-next-line no-param-reassign - options.content = `${hashPrefix + request}\x00${unescape(localName)}`; + options.content = `${hashPrefix + request}\x00${localName}`; - // Using `[path]` placeholder outputs `/` we need escape their - // Also directories can contains invalid characters for css we need escape their too - return cssesc( - interpolateName(loaderContext, localIdentName, options) - // For `[hash]` placeholder - .replace(/^((-?[0-9])|--)/, '_$1') - .replace(filenameReservedRegex, '-') - .replace(reControlChars, '-') - .replace(/\./g, '-'), - { isIdentifier: true } - ).replace(/\\\[local\\]/gi, localName); + return interpolateName(loaderContext, localIdentName, options); } function normalizeUrl(url, isStringValue) { @@ -143,7 +145,8 @@ function getModulesOptions(rawOptions, loaderContext) { localIdentHashPrefix: '', // eslint-disable-next-line no-undefined localIdentRegExp: undefined, - getLocalIdent: defaultGetLocalIdent, + // eslint-disable-next-line no-undefined + getLocalIdent: undefined, namedExport: false, exportLocalsConvention: 'asIs', exportOnlyLocals: false, @@ -281,32 +284,42 @@ function getModulesPlugins(options, loaderContext) { extractImports(), modulesScope({ generateScopedName(exportName) { - let localIdent = getLocalIdent( - loaderContext, - localIdentName, - exportName, - { - context: localIdentContext, - hashPrefix: localIdentHashPrefix, - regExp: localIdentRegExp, - } - ); + let localIdent; + + if (typeof getLocalIdent !== 'undefined') { + localIdent = getLocalIdent( + loaderContext, + localIdentName, + unescape(exportName), + { + context: localIdentContext, + hashPrefix: localIdentHashPrefix, + regExp: localIdentRegExp, + } + ); + } // A null/undefined value signals that we should invoke the default // getLocalIdent method. - if (localIdent == null) { + if (typeof localIdent === 'undefined' || localIdent === null) { localIdent = defaultGetLocalIdent( loaderContext, localIdentName, - exportName, + unescape(exportName), { context: localIdentContext, hashPrefix: localIdentHashPrefix, regExp: localIdentRegExp, } ); + + return escapeLocalident(localIdent).replace( + /\\\[local\\]/gi, + exportName + ); } - return localIdent; + + return escapeLocalident(localIdent); }, exportGlobals: options.modules.exportGlobals, }), diff --git a/test/__snapshots__/modules-option.test.js.snap b/test/__snapshots__/modules-option.test.js.snap index da337d58..c50f1321 100644 --- a/test/__snapshots__/modules-option.test.js.snap +++ b/test/__snapshots__/modules-option.test.js.snap @@ -137,6 +137,58 @@ Array [ exports[`"modules" option issue #861: warnings 1`] = `Array []`; +exports[`"modules" option issue #966 - values in selectors aren't escaped properly: errors 1`] = `Array []`; + +exports[`"modules" option issue #966 - values in selectors aren't escaped properly: module 1`] = ` +"// Imports +import ___CSS_LOADER_API_IMPORT___ from \\"../../../../src/runtime/api.js\\"; +var ___CSS_LOADER_EXPORT___ = ___CSS_LOADER_API_IMPORT___(false); +// Module +___CSS_LOADER_EXPORT___.push([module.id, \\"._7-foo-class {\\\\n color: red;\\\\n}\\\\n\\\\n.\\\\\\\\--bar-class {\\\\n color: red;\\\\n}\\\\n\\\\n.\\\\\\\\--baz-class {\\\\n color: red;\\\\n}\\\\n\\\\n.fooBaz-class-continuation {\\\\n color: red;\\\\n}\\\\n\\\\n.some.class {\\\\n color: red;\\\\n}\\\\n\\", \\"\\"]); +// Exports +___CSS_LOADER_EXPORT___.locals = { + \\"foo-class\\": \\"_7-foo-class\\", + \\"bar-class\\": \\"--bar-class\\", + \\"baz-class\\": \\"--baz-class\\", + \\"fooBaz-class\\": \\"fooBaz-class-continuation\\", + \\"some\\": \\"some\\", + \\"class\\": \\"class\\" +}; +export default ___CSS_LOADER_EXPORT___; +" +`; + +exports[`"modules" option issue #966 - values in selectors aren't escaped properly: result 1`] = ` +Array [ + Array [ + "./modules/issue-966/issue-966.css", + "._7-foo-class { + color: red; +} + +.\\\\--bar-class { + color: red; +} + +.\\\\--baz-class { + color: red; +} + +.fooBaz-class-continuation { + color: red; +} + +.some.class { + color: red; +} +", + "", + ], +] +`; + +exports[`"modules" option issue #966 - values in selectors aren't escaped properly: warnings 1`] = `Array []`; + exports[`"modules" option issue #966: errors 1`] = `Array []`; exports[`"modules" option issue #966: module 1`] = ` @@ -144,10 +196,10 @@ exports[`"modules" option issue #966: module 1`] = ` import ___CSS_LOADER_API_IMPORT___ from \\"../../../../src/runtime/api.js\\"; var ___CSS_LOADER_EXPORT___ = ___CSS_LOADER_API_IMPORT___(false); // Module -___CSS_LOADER_EXPORT___.push([module.id, \\".button.hey {\\\\n color: red;\\\\n}\\\\n\\", \\"\\"]); +___CSS_LOADER_EXPORT___.push([module.id, \\".button-hey {\\\\n color: red;\\\\n}\\\\n\\", \\"\\"]); // Exports ___CSS_LOADER_EXPORT___.locals = { - \\"button\\": \\"button.hey\\" + \\"button\\": \\"button-hey\\" }; export default ___CSS_LOADER_EXPORT___; " @@ -157,7 +209,7 @@ exports[`"modules" option issue #966: result 1`] = ` Array [ Array [ "./modules/issue-966/button.css", - ".button.hey { + ".button-hey { color: red; } ", diff --git a/test/fixtures/modules/issue-966/issue-966.css b/test/fixtures/modules/issue-966/issue-966.css new file mode 100644 index 00000000..a27ca561 --- /dev/null +++ b/test/fixtures/modules/issue-966/issue-966.css @@ -0,0 +1,19 @@ +.foo-class { + color: red; +} + +.bar-class { + color: red; +} + +.baz-class { + color: red; +} + +.fooBaz-class { + color: red; +} + +.some.class { + color: red; +} diff --git a/test/fixtures/modules/issue-966/issue-966.js b/test/fixtures/modules/issue-966/issue-966.js new file mode 100644 index 00000000..99ef679a --- /dev/null +++ b/test/fixtures/modules/issue-966/issue-966.js @@ -0,0 +1,5 @@ +import css from './issue-966.css'; + +__export__ = css; + +export default css; diff --git a/test/modules-option.test.js b/test/modules-option.test.js index 0a7f4b02..06b7cd27 100644 --- a/test/modules-option.test.js +++ b/test/modules-option.test.js @@ -491,6 +491,45 @@ describe('"modules" option', () => { expect(getErrors(stats)).toMatchSnapshot('errors'); }); + it("issue #966 - values in selectors aren't escaped properly", async () => { + const compiler = getCompiler('./modules/issue-966/issue-966.js', { + modules: { + getLocalIdent: (loaderContext, localIdentName, localName) => { + if (localName === 'foo-class') { + return `7-${localName}`; + } + + if (localName === 'bar-class') { + return `>-${localName}`; + } + + if (localName === 'baz-class') { + return `\u0000-${localName}`; + } + + if (localName === 'fooBaz-class') { + return `${localName}.continuation`; + } + + return null; + }, + localIdentName: '[local]', + }, + }); + const stats = await compile(compiler); + + expect( + getModuleSource('./modules/issue-966/issue-966.css', stats) + ).toMatchSnapshot('module'); + + expect(getExecutedCode('main.bundle.js', compiler, stats)).toMatchSnapshot( + 'result' + ); + + expect(getWarnings(stats)).toMatchSnapshot('warnings'); + expect(getErrors(stats)).toMatchSnapshot('errors'); + }); + it('issue #967', async () => { const compiler = getCompiler('./modules/issue-967/path-placeholder.js', { modules: {