-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
perf: regexp perf issues, refactor regexp stylistic issues #10905
Conversation
@@ -40,7 +40,7 @@ const htmlTypesRE = /\.(html|vue|svelte|astro|imba)$/ | |||
// since even missed imports can be caught at runtime, and false positives will | |||
// simply be ignored. | |||
export const importsRE = | |||
/(?<!\/\/.*)(?<=^|;|\*\/)\s*import(?!\s+type)(?:[\w*{}\n\r\t, ]+from\s*)?\s*("[^"]+"|'[^']+')\s*(?=$|;|\/\/|\/\*)/gm | |||
/(?<!\/\/.*)(?<=^|;|\*\/)\s*import(?!\s+type)(?:[\w*{}\n\r\t, ]+from)?\s*("[^"]+"|'[^']+')\s*(?=$|;|\/\/|\/\*)/gm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This \s*
can be matched by the next \s*
.
@@ -149,13 +149,13 @@ function globEntries(pattern: string | string[], config: ResolvedConfig) { | |||
} | |||
|
|||
const scriptModuleRE = | |||
/(<script\b[^>]*type\s*=\s*(?:"module"|'module')[^>]*>)(.*?)<\/script>/gims | |||
export const scriptRE = /(<script\b(?:\s[^>]*>|>))(.*?)<\/script>/gims | |||
/(<script\b[^>]+type\s*=\s*(?:"module"|'module')[^>]*>)(.*?)<\/script>/gis |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because \b
exists [^>]
will always match more than one char.
/(<script\b[^>]*type\s*=\s*(?:"module"|'module')[^>]*>)(.*?)<\/script>/gims | ||
export const scriptRE = /(<script\b(?:\s[^>]*>|>))(.*?)<\/script>/gims | ||
/(<script\b[^>]+type\s*=\s*(?:"module"|'module')[^>]*>)(.*?)<\/script>/gis | ||
export const scriptRE = /(<script(?:\s[^>]*>|>))(.*?)<\/script>/gis |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because \s
and >
both matches \b
, this \b
isn't needed.
@@ -38,7 +38,7 @@ export function assetImportMetaUrlPlugin(config: ResolvedConfig): Plugin { | |||
) { | |||
let s: MagicString | undefined | |||
const assetImportMetaUrlRE = | |||
/\bnew\s+URL\s*\(\s*('[^']+'|"[^"]+"|`[^`]+`)\s*,\s*import\.meta\.url\s*,?\s*\)/g | |||
/\bnew\s+URL\s*\(\s*('[^']+'|"[^"]+"|`[^`]+`)\s*,\s*import\.meta\.url\s*(?:,\s*)?\)/g |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\s*,?\s*
is same with \s*(?:,\s*)?
.
/(?<=^|[^\w\-\u0080-\uffff])url\(\s*('[^']+'|"[^"]+"|[^'")]+)\s*\)/ | ||
/(?<=^|[^\w\-\u0080-\uffff])url\((\s*('[^']+'|"[^"]+")\s*|[^'")]+)\)/ | ||
export const cssDataUriRE = | ||
/(?<=^|[^\w\-\u0080-\uffff])data-uri\(\s*('[^']+'|"[^"]+"|[^'")]+)\s*\)/ | ||
/(?<=^|[^\w\-\u0080-\uffff])data-uri\((\s*('[^']+'|"[^"]+")\s*|[^'")]+)\)/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\s*('[^']+'|"[^"]+"|[^'")]+)\s*
is same with (\s*('[^']+'|"[^"]+")\s*|[^'")]+)
. (String::trim()
needs to be run later.)
/@import\s*(?:url\([^\)]*\)|"([^"]|(?<=\\)")*"|'([^']|(?<=\\)')*'|[^;]*).*?;/gm | ||
/@import(?:\s*(?:url\([^)]*\)|"(?:[^"]|(?<=\\)")*"|'(?:[^']|(?<=\\)')*').*?|[^;]*);/g |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[^;]*
matches \s*
. So splitted [^;]*
.
@@ -20,7 +20,7 @@ export function ssrRewriteStacktrace( | |||
.split('\n') | |||
.map((line) => { | |||
return line.replace( | |||
/^ {4}at (?:(.+?)\s+\()?(?:(.+?):(\d+)(?::(\d+))?)\)?/, | |||
/^ {4}at (?:(\S.*?)\s\()?(.+?):(\d+)(?::(\d+))?\)?/, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(.+?)\s+
is same with (\S.*?)\s
. (String::trim()
needs to be run later.)
Description
Add
eslint-plugin-regexp
to detect some perf issues.I guess this will fix #10900 along the way.Actually it wasn't catched byeslint-plugin-regexp
. But I included a fix for it in this PR.Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).