Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix escape getLocalIdent #1196

Merged
merged 3 commits into from
Sep 24, 2020
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
65 changes: 39 additions & 26 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
}),
Expand Down
58 changes: 55 additions & 3 deletions test/__snapshots__/modules-option.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -137,17 +137,69 @@ 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`] = `
"// 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, \\".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___;
"
Expand All @@ -157,7 +209,7 @@ exports[`"modules" option issue #966: result 1`] = `
Array [
Array [
"./modules/issue-966/button.css",
".button.hey {
".button-hey {
color: red;
}
",
Expand Down
19 changes: 19 additions & 0 deletions test/fixtures/modules/issue-966/issue-966.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
.foo-class {
color: red;
}

.bar-class {
color: red;
}

.baz-class {
color: red;
}

.fooBaz-class {
color: red;
}

.some.class {
color: red;
}
5 changes: 5 additions & 0 deletions test/fixtures/modules/issue-966/issue-966.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import css from './issue-966.css';

__export__ = css;

export default css;
39 changes: 39 additions & 0 deletions test/modules-option.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down