Skip to content

fix(babel-highlight::index.ts): Short-circuit empty source highlighting - #14381

Closed
a20185 wants to merge 3 commits into
babel:mainfrom
a20185:patch-1
Closed

a20185 wants to merge 3 commits into
babel:mainfrom
a20185:patch-1

Conversation

@a20185

@a20185 a20185 commented Mar 22, 2022 •

Copy link
Copy Markdown

Short-circuit empty source highlighting to prevent infinite iteration.

Q                       A
Fixed Issues? NONE
Patch: Bug Fix? Yes
Major: Breaking Change? No
Minor: New Feature? No
Tests Added + Pass? Yes
Documentation PR Link No
Any Dependency Changes? No
License MIT

When we are using babel-highlight to highlight the stylelint output results, we surprisingly found that babel-highlight plugin fails on empty source files: While passing '' to tokenize function, it will end up in an infinite iterator.

IMO I thought the empty source is worthless for highlighting(since the result won't disappear to be different after highlighting), so maybe a short-circuit guard will be a solution to this case 😄

FYI,the case we've encountered in this situation:
image

Short-circuit empty source highlighting to prevent infinite iteration.
@babel-bot

babel-bot commented Mar 22, 2022 •

Copy link
Copy Markdown
Collaborator

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/51536/

Add comma to match project lint errors
@a20185

a20185 commented Mar 22, 2022

Copy link
Copy Markdown
Author

@nicolo-ribaudo @zxbodya @CommanderRoot @blankPen
If you have time, please help to check if this modification is okay.
It's really nesassary for us, thanks a lot! ❤

@JLHwung

JLHwung commented Mar 22, 2022

Copy link
Copy Markdown
Contributor

We already guard against the empty source in #14165. The highlightTokens is an internal function, I don't think we need to check that again. Can you post your use case?

@a20185

a20185 commented Mar 23, 2022

Copy link
Copy Markdown
Author

We already guard against the empty source in #14165. The highlightTokens is an internal function, I don't think we need to check that again. Can you post your use case?

Ah,#14165 should solve the problem we've encountered, and seemed to be a better solution.

The situation we've been in is that we've using stylelint for style linting in vue-cli. When calling vue-cli-service lint:style, it will call stylelint to lint scss files.

When we've create empty files in the project, the stylelint creates warning for empty file, showing that it conflicts with no-empty-source, using babel-highlight for highlighting outputs, passing '' to the highlighter, and eventually trapped in the infinite loop of highlightTokens.

IMO this is nesassary for us, since it's not only awkward for not knowing what REALLY HAPPENS in linting styles by actually shows nothing but a forever spinning icon, but also stuck our building CI/CD procedure.

Really looking forward for the bugfix releases, can you please to help for a faster progress?

@a20185

a20185 commented Mar 23, 2022

Copy link
Copy Markdown
Author

This PR should be closed since #14165 could be a better solution.

@nicolo-ribaudo

Copy link
Copy Markdown
Member

It has been released in v7.16.10 🤔

@a20185 a20185 closed this Mar 23, 2022
@a20185

a20185 commented Mar 23, 2022 •

Copy link
Copy Markdown
Author

It has been released in v7.16.10 🤔

Thanks! ❤

@github-actions github-actions Bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jun 23, 2022
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants