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.

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

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

Closed
wants to merge 3 commits into from

Conversation

a20185
Copy link

@a20185 a20185 commented Mar 22, 2022

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
Copy link
Collaborator

babel-bot commented Mar 22, 2022

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
Copy link
Author

a20185 commented Mar 22, 2022

@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
Copy link
Contributor

JLHwung commented Mar 22, 2022

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
Copy link
Author

a20185 commented Mar 23, 2022

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
Copy link
Author

a20185 commented Mar 23, 2022

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

@nicolo-ribaudo
Copy link
Member

It has been released in v7.16.10 🤔

@a20185 a20185 closed this Mar 23, 2022
@a20185
Copy link
Author

a20185 commented Mar 23, 2022

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.

None yet

4 participants