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

feat(Sort): add sort package #967

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(Sort): add sort package #967

wants to merge 1 commit into from

Conversation

allison-c
Copy link
Contributor

Purpose 🚀

clear, one sentence description of primary purpose of this PR

Notes ✏️

details of code change / secondary purposes of this PR

Updates 📦

If you have changed a component's source code (not stories, specs, or docs), before merging your branch run yarn changeset. This will prompt you to:

  • indicate if changes were patch/minor/major for each modified package
  • enter a release message

Storybook 📕

http://storybooks.highbond-s3.com/paprika/your-branch-name

Screenshots 📸

optional but highly recommended

References 🔗

relevant Jira ticket / GitHub issues

@allison-c allison-c added the WIP label Mar 9, 2021
@changeset-bot
Copy link

changeset-bot bot commented Mar 9, 2021

⚠️ No Changeset found

Latest commit: 1c92f47

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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


export default function getAlphaSortFunction(locale) {
if (!alphaSortCompareFunction) {
alphaSortCompareFunction = getCompareFunc(locale);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it expensive to create a collator, hence the laziness?
Asking because I think there's a bug here: once you've called getAlphaSortFunction with a locale (e.g. 'en'), all further calls will get you a function for the same locale, even if you're passing something different (e.g. 'de').

Also alphaSortAsc seems redundant, couldn't this just return alphaSortCompareFunction?

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 don't recall why we memorize alphaSortCompareFunction... seems like collator performance is good enough, but I understand the bug you brought up, I'll create an object to memorize the method for different locals

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants