Avoid infinite loop when highlighting an empty input#14165
Avoid infinite loop when highlighting an empty input#14165nicolo-ribaudo merged 2 commits intobabel:mainfrom
Conversation
fix(highlight): When s is an empty string, highlight will die cycle
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/50797/ |
The-x-Theorist
left a comment
There was a problem hiding this comment.
It wil good if it gets change if (shouldHighlight(options)) { to if (shouldHighlight(options) && code) {
| * Highlight `code`. | ||
| */ | ||
| export default function highlight(code: string, options: Options = {}): string { | ||
| if (!code) return code; |
There was a problem hiding this comment.
| if (!code) return code; |
There was a problem hiding this comment.
This should be removed.
| */ | ||
| export default function highlight(code: string, options: Options = {}): string { | ||
| if (!code) return code; | ||
| if (shouldHighlight(options)) { |
There was a problem hiding this comment.
| if (shouldHighlight(options)) { | |
| if (shouldHighlight(options) && code) { |
There was a problem hiding this comment.
This would look more clean.
There was a problem hiding this comment.
It suffices to check empty string only code !== "" because regexp#exec always return non-null results for empty string.
There was a problem hiding this comment.
Yeah, empty string check will be more efficient and safe.
| */ | ||
| export default function highlight(code: string, options: Options = {}): string { | ||
| if (!code) return code; | ||
| if (shouldHighlight(options)) { |
There was a problem hiding this comment.
This would look more clean.
| * Highlight `code`. | ||
| */ | ||
| export default function highlight(code: string, options: Options = {}): string { | ||
| if (!code) return code; |
There was a problem hiding this comment.
This should be removed.
nicolo-ribaudo
left a comment
There was a problem hiding this comment.
I'm fine with the current approach, but feel free to merge the two if statements. However, if you merge them the check should be code !== "" && shouldHighlight(options) rather than shouldHighlight(options) && code !== "", because the code !== "" check is less expensive.
|
Thanks for your PR! As a suggestion for the future, avoid opening PR from the |
fix(highlight): When text is an empty string, highlight will die cycle