Skip to content

refactor: drop lodash#210

Merged
ota-meshi merged 3 commits intoota-meshi:masterfrom
SukkaW:drop-lodash
Feb 28, 2025
Merged

refactor: drop lodash#210
ota-meshi merged 3 commits intoota-meshi:masterfrom
SukkaW:drop-lodash

Conversation

@SukkaW
Copy link
Contributor

@SukkaW SukkaW commented Feb 18, 2025

See also ota-meshi/eslint-plugin-yml#406

yaml-eslint-parser is the last package in my eslint-config-sukka that depends on lodash.

It turns out that we actually don't even need sortedLastIndex at all. Instead, I replace it with a modified version of ESLint's implementation of context.sourceCode.getLocFromIndex, and all tests still pass on my machine:

image

src/context.ts Outdated
const lineNumber =
lastIndice != null && index >= lastIndice
? this.lineStartIndices.length
: this.lineStartIndices.findIndex((el) => index < el);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR!
When I tried it previously, I found that this change had worse performance than sortedLastIndex.
ESLint core only makes limited use of sortedLastIndex, but this package is called for every node and token.
Could you change the search logic to be more similar to lodash?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have thought of switch lodash to remeda: https://github.com/remeda/remeda/blob/main/src/sortedLastIndex.ts

But remeda has an installation size of 2 MiB which is still not ideal.

I have also thought of the lodash.sortedlastindex standalone package.

Maybe I should just copy the O(log n) implementation from remeda.

Copy link
Contributor Author

@SukkaW SukkaW Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ota-meshi I have added the implementation of sortedlastindex in 0aed79e (#210).

All tests still pass locally:

image

@changeset-bot
Copy link

changeset-bot bot commented Feb 25, 2025

🦋 Changeset detected

Latest commit: 0aed79e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
yaml-eslint-parser Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@SukkaW SukkaW requested a review from ota-meshi February 25, 2025 02:08
Copy link
Owner

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much!

@ota-meshi ota-meshi merged commit 96b72da into ota-meshi:master Feb 28, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants