[Fix] dynamic-import-chunkname: allow single quotes to match Webpack support
#1848
Conversation
There was a problem hiding this comment.
I'm concerned about webpack users prior to v4.43.0 getting false positives here.
I think the safest approach is an option, that defaults to false, but can be set to "detect" or true. When "detect", it could try/catch-require webpack and check the version number.
| const commentStyleRegex = /^( \w+: (["'][^"']*["']|\d+|false|true),?)+ $/ | ||
| const chunkSubstrFormat = ` webpackChunkName: ["']${webpackChunknameFormat}["'],? ` |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
It's inside a comment tho, so I'm not sure why it'd be invalid syntax?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
I'd prefer the backreferences, but it doesn't need to block the PR.
2 similar comments
Sorry for the confusion here, I only meant to confirm that single quotes are supported in 4.43.0 (their latest as of now), but I'm sure the support extends back farther than that since I've used single quotes for some time. It struck me as tangential to the purpose of this rule about the format of the chunkName to get too deep in the weeds of Webpack support, but I see what you're saying. If you think it's important, I can attempt to find out how far back Webpack support for single quotes goes. |
|
I ran a quick test, and the single quotes are supported in Webpack all the way back to v2.4.0, which as I understand it is the release that introduced magic comments. Given that, there wouldn't seem much value in a Please let me know how you'd like to proceed, and thanks for your feedback so far! |
|
I totally agree, thanks for doing the research. |
|
@ljharb Did you still want me to pursue adding a |
|
@straub sorry for the delay :-) let me take a fresh look now. |
| const commentStyleRegex = /^( \w+: (["'][^"']*["']|\d+|false|true),?)+ $/ | ||
| const chunkSubstrFormat = ` webpackChunkName: ["']${webpackChunknameFormat}["'],? ` |
There was a problem hiding this comment.
I'd prefer the backreferences, but it doesn't need to block the PR.
ljharb
force-pushed
the
fix/dynamic-import-chunkname-single-quotes
branch
from
274b252
to
227d9a2
Compare
ljharb
changed the title
dynamic-import-chunkname: allow single quotes to match Webpack support
Single quotes are supported by Webpack at least in v4.43.0, which is the
version I'm currently using. Since my eslint style prefers single
quotes, I naturally wrote my Webpack magic comments to match, which
worked w/o issue until I tried to enable this rule.
Implemented as a non-breaking relaxation of the existing rule to avoid
complexity per
#1130 (comment)
but please let me know if you prefer a different approach!
Fixes #1130.