-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Update: deprecate sourceCode#isSpaceBetweenTokens() #12519
Conversation
TSC SummaryWhile fixing a bug in in TSC QuestionIn addition to the bug fix/documentation corrections in #12491, I'd like to propose we deprecate This PR currently deprecates |
This was accepted at yesterday's TSC meeting. |
c48c806
to
0c632a0
Compare
@@ -1789,6 +1789,339 @@ describe("SourceCode", () => { | |||
}); | |||
}); | |||
|
|||
describe("isSpaceBetween()", () => { |
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.
Duplicating the tests below for isSpaceBetweenTokens()
.
This has been rebased and is ready for review. |
What is the purpose of this pull request? (put an "X" next to item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[X] Add something to the core
[ ] Other, please explain:
Please evaluate just the last commit. We should wait to merge this until after we land #12491. I'll rebase at that time. Adding the "do not merge" label until then.
What changes did you make? (Give an overview)
While working on #12491, noticed that
sourceCode#isSpaceBetweenTokens()
currently allows for nodes as well as tokens as arguments. I think that it would make sense to rename this method to isSpaceBetween() and leave the ability for it to check between nodes and tokens, as that's a useful behavior that is most likely being used by custom rules (since we use it in at least one of our core rules). We could deprecate isSpaceBetweenTokens() and leave it as an alias that calls through to isSpaceBetween() for the sake of backwards compatibility.Is there anything you'd like reviewers to focus on?
Does this API make sense?