Skip to content

Extract LookupByPath -> @rushstack/lookup-by-path #4868

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

Merged
merged 3 commits into from
Aug 8, 2024

Conversation

dmichon-msft
Copy link
Contributor

@dmichon-msft dmichon-msft commented Aug 7, 2024

Summary

Extracts the LookupByPath class into its own package, @rushstack/lookup-by-path.

Renames the class to PathTrie to reflect the standard computer science name for the structure.

The motivation for this PR is to be able to reuse this class in the upcoming @rushstack/webpack-workspace-resolve-plugin, or in other packages that cannot take a dependency on @microsoft/rush-lib or @rushstack/rush-sdk.

Details

Removes the ability to deep import @microsoft/rush-lib/lib/logic/LookupByPath.

How it was tested

Unit tests.

Impacted documentation

API spec.

@octogonz
Copy link
Collaborator

octogonz commented Aug 8, 2024

I'm okay with this, however... despite being "standard" among academics, I find the name "trie" to be annoying because its pronunciation is confusingly identical to "tree". This jargon will be unfamiliar to most engineers, which may hinder adoption. The other common name "prefix tree" is much more intuitive, but the name doesn't /have/ to be the implementation. If instead we name the usage/purpose, the name could be like "PathLookup" etc

@octogonz
Copy link
Collaborator

octogonz commented Aug 8, 2024

Oh hey, LookupByPath is pretty good 😄

@dmichon-msft dmichon-msft changed the title Extract LookupByPath -> @rushstack/path-trie Extract LookupByPath -> @rushstack/lookup-by-path Aug 8, 2024
@dmichon-msft dmichon-msft merged commit 886111d into main Aug 8, 2024
5 checks passed
@dmichon-msft dmichon-msft deleted the user/dmichon/path-trie branch August 8, 2024 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

3 participants