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

Replace custom "findSuggestion" function with "levenary" #10924

Merged
merged 2 commits into from Dec 25, 2019

Conversation

nicolo-ribaudo
Copy link
Member

Q                       A
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes? Yes
License MIT

This PR is extracted from #10899. In #10899 I needed to use the findSuggestion function in two different libraries, so I would have had to duplicate that code.
Instead of duplicating it, I decided to dig into the universe of fastest levenshtein distance functions on npm to find one with the interface that we actually need.
I initially used didyoumean2, but then switched to levenary because:

  1. it's faster and because 😉
  2. it's created by @tanhauhau
  3. it provides flow typings out of the box.

I extracted this commit to a separate PR because @JLHwung is going to work on some content which touches similar lines/depends on this package, so we can merge this without waiting for #10899.

@nicolo-ribaudo nicolo-ribaudo added PR: Internal 🏠 A type of pull request used for our changelog categories PR: New Dependency pkg: preset-env labels Dec 25, 2019
@tanhauhau
Copy link
Member

Published levenary@1.1.0 with @JLHwung's PR tanhauhau/levenary#1

@nicolo-ribaudo nicolo-ribaudo merged commit e1b01cd into babel:master Dec 25, 2019
@nicolo-ribaudo nicolo-ribaudo deleted the leven branch December 25, 2019 23:44
nicolo-ribaudo added a commit that referenced this pull request Dec 25, 2019
* Replace custom "findSuggestion" function with "levenary"

* Update
@nicolo-ribaudo nicolo-ribaudo changed the title Replace custom "fundSuggestion" function with "levenary" Replace custom "findSuggestion" function with "levenary" Dec 25, 2019
@SimenB
Copy link
Contributor

SimenB commented Jan 27, 2020

This is a breaking change - levenary depends on Node 8.

(I realise node 6 (& 8) is EOL - this is still a breaking change in a minor release)

@SimenB
Copy link
Contributor

SimenB commented Jan 27, 2020

leven itself is node 6, so it should just be a matter of downgrading the requirement in levenary: tanhauhau/levenary#3. Hopefully that's all that's needed

@nicolo-ribaudo
Copy link
Member Author

Thanks for catching this! cc @tanhauhau

@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 Apr 28, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 28, 2020
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 pkg: preset-env PR: Internal 🏠 A type of pull request used for our changelog categories PR: New Dependency
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants