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.

Sign up for GitHub

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

Jump to bottom

[Fix] dynamic-import-chunkname: allow single quotes to match Webpack support #1848

Merged
merged 1 commit into from Aug 30, 2020
Merged

[Fix] dynamic-import-chunkname: allow single quotes to match Webpack support #1848

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -10,6 +10,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- [`default`]/TypeScript: avoid crash on `export =` with a MemberExpression ([#1841], thanks [@ljharb])
- [`extensions`]/importType: Fix @/abc being treated as scoped module ([#1854], thanks [@3nuc])
- allow using rest operator in named export ([#1878], thanks [@foray1010])
- [`dynamic-import-chunkname`]: allow single quotes to match Webpack support ([#1848], thanks [@straub])

### Changed
- [`export`]: add tests for a name collision with `export * from` ([#1704], thanks @tomprats)
Expand Down Expand Up @@ -732,6 +733,7 @@ for info on changes for earlier releases.

[#1878]: https://github.com/benmosher/eslint-plugin-import/pull/1878
[#1854]: https://github.com/benmosher/eslint-plugin-import/issues/1854
[#1848]: https://github.com/benmosher/eslint-plugin-import/pull/1848
[#1841]: https://github.com/benmosher/eslint-plugin-import/issues/1841
[#1836]: https://github.com/benmosher/eslint-plugin-import/pull/1836
[#1835]: https://github.com/benmosher/eslint-plugin-import/pull/1835
Expand Down Expand Up @@ -1271,3 +1273,4 @@ for info on changes for earlier releases.
[@3nuc]: https://github.com/3nuc
[@foray1010]: https://github.com/foray1010
[@tomprats]: https://github.com/tomprats
[@straub]: https://github.com/straub
12 changes: 6 additions & 6 deletions docs/rules/dynamic-import-chunkname.md
View file
Open in desktop
Expand Up @@ -39,12 +39,6 @@ import(
'someModule',
);

// using single quotes instead of double quotes
import(
/* webpackChunkName: 'someModule' */
'someModule',
);

// invalid syntax for webpack comment
import(
/* totally not webpackChunkName: "someModule" */
Expand Down Expand Up @@ -78,6 +72,12 @@ The following patterns are valid:
/* webpackChunkName: "someModule", webpackPrefetch: true */
'someModule',
);

// using single quotes instead of double quotes
import(
/* webpackChunkName: 'someModule' */
'someModule',
);
```

## When Not To Use It
Expand Down
4 changes: 2 additions & 2 deletions src/rules/dynamic-import-chunkname.js
View file
Open in desktop
Expand Up @@ -30,8 +30,8 @@ module.exports = {
const { webpackChunknameFormat = '[0-9a-zA-Z-_/.]+' } = config || {}

const paddedCommentRegex = /^ (\S[\s\S]+\S) $/
const commentStyleRegex = /^( \w+: ("[^"]*"|\d+|false|true),?)+ $/
const chunkSubstrFormat = ` webpackChunkName: "${webpackChunknameFormat}",? `
const commentStyleRegex = /^( \w+: (["'][^"']*["']|\d+|false|true),?)+ $/
const chunkSubstrFormat = ` webpackChunkName: ["']${webpackChunknameFormat}["'],? `
Comment on lines +33 to +34
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these don't use backreferences, so mismatched quotes would be supported. I see the test cases that suggest they aren't supported, but i'm not clear on how that works. can you help me understand?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the same thought at first! The gist is that mismatched quotes are invalid syntax, so they trigger the syntax error and don't need to be checked in the style regex. I can use backreferences in these patterns instead, if you prefer to have it covered both ways, just lmk :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's inside a comment tho, so I'm not sure why it'd be invalid syntax?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment value is parsed as the interior of a JS object literal, in order to validate the syntax: https://github.com/benmosher/eslint-plugin-import/blob/274b252138aa6c268db9084875f024b45c4eb9c5/src/rules/dynamic-import-chunkname.js#L72

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer the backreferences, but it doesn't need to block the PR.

const chunkSubstrRegex = new RegExp(chunkSubstrFormat)

function run(node, arg) {
Expand Down
79 changes: 56 additions & 23 deletions tests/src/rules/dynamic-import-chunkname.js
View file
Open in desktop
Expand Up @@ -21,8 +21,8 @@ const noLeadingCommentError = 'dynamic imports require a leading comment with th
const nonBlockCommentError = 'dynamic imports require a /* foo */ style comment, not a // foo comment'
const noPaddingCommentError = 'dynamic imports require a block comment padded with spaces - /* foo */'
const invalidSyntaxCommentError = 'dynamic imports require a "webpack" comment with valid syntax'
const commentFormatError = `dynamic imports require a leading comment in the form /* webpackChunkName: "${commentFormat}",? */`
const pickyCommentFormatError = `dynamic imports require a leading comment in the form /* webpackChunkName: "${pickyCommentFormat}",? */`
const commentFormatError = `dynamic imports require a leading comment in the form /* webpackChunkName: ["']${commentFormat}["'],? */`
const pickyCommentFormatError = `dynamic imports require a leading comment in the form /* webpackChunkName: ["']${pickyCommentFormat}["'],? */`

ruleTester.run('dynamic-import-chunkname', rule, {
valid: [
Expand Down Expand Up @@ -132,6 +132,14 @@ ruleTester.run('dynamic-import-chunkname', rule, {
options,
parser,
},
{
code: `import(
/* webpackChunkName: 'someModule' */
'someModule'
)`,
options,
parser,
},
{
code: `import(
/* webpackChunkName: "someModule" */
Expand Down Expand Up @@ -192,17 +200,33 @@ ruleTester.run('dynamic-import-chunkname', rule, {
},
{
code: `import(
/* webpackChunkName: 'someModule' */
/* webpackChunkName: "someModule' */
'someModule'
)`,
options,
parser,
output: `import(
/* webpackChunkName: 'someModule' */
/* webpackChunkName: "someModule' */
'someModule'
)`,
errors: [{
message: commentFormatError,
message: invalidSyntaxCommentError,
type: 'CallExpression',
}],
},
{
code: `import(
/* webpackChunkName: 'someModule" */
'someModule'
)`,
options,
parser,
output: `import(
/* webpackChunkName: 'someModule" */
'someModule'
)`,
errors: [{
message: invalidSyntaxCommentError,
type: 'CallExpression',
}],
},
Expand Down Expand Up @@ -421,21 +445,6 @@ ruleTester.run('dynamic-import-chunkname', rule, {
type: 'CallExpression',
}],
},
{
code: `dynamicImport(
/* webpackChunkName: 'someModule' */
'someModule'
)`,
options,
output: `dynamicImport(
/* webpackChunkName: 'someModule' */
'someModule'
)`,
errors: [{
message: commentFormatError,
type: 'CallExpression',
}],
},
{
code: `dynamicImport(
/* webpackChunkName "someModule" */
Expand Down Expand Up @@ -578,6 +587,14 @@ context('TypeScript', () => {
type: nodeType,
}],
},
{
code: `import(
/* webpackChunkName: 'someModule' */
'test'
)`,
options,
parser: typescriptParser,
},
],
invalid: [
{
Expand Down Expand Up @@ -624,17 +641,33 @@ context('TypeScript', () => {
},
{
code: `import(
/* webpackChunkName: 'someModule' */
/* webpackChunkName "someModule' */
'someModule'
)`,
options,
parser: typescriptParser,
output: `import(
/* webpackChunkName: 'someModule' */
/* webpackChunkName "someModule' */
'someModule'
)`,
errors: [{
message: commentFormatError,
message: invalidSyntaxCommentError,
type: nodeType,
}],
},
{
code: `import(
/* webpackChunkName 'someModule" */
'someModule'
)`,
options,
parser: typescriptParser,
output: `import(
/* webpackChunkName 'someModule" */
'someModule'
)`,
errors: [{
message: invalidSyntaxCommentError,
type: nodeType,
}],
},
Expand Down