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

Remove unused Intl.Segmenter polyfill #1789

Merged
merged 1 commit into from
Nov 1, 2023
Merged

Remove unused Intl.Segmenter polyfill #1789

merged 1 commit into from
Nov 1, 2023

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Nov 1, 2023

Closes #718.

The graphemer dep and Intl polyfill were originally added in 1b93986 for composer (4549893).

However, @atproto/api no longer uses Intl and instead uses graphemer directly: bluesky-social/atproto#1044. So there is no need for these.

Test Plan

Verified composer works on iOS, Android, and web (Chrome and FF).

@gaearon gaearon requested a review from pfrazee November 1, 2023 16:46
// loading emoji mart data
// TODO: This condition doesn't make sense; emojimart has nothing to do with Intl.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Accidentally noticed this. I presume it was added without looking at the condition above. What's happening here is bizarre:

  • Emojimart data is undefined in local development (because the condition never triggers) but the emoji picker somehow works. (Is data not required??)
  • In production, emojimart data is defined, but it's only because our scripts.html partial template adds a <script> tag for the thing that triggers emojimart data loading anyway. So this code isn't necessary.

I'd remove it now but I want to understand why it works without data locally. So for now, not touching this.

@gaearon gaearon merged commit 9fb2051 into main Nov 1, 2023
4 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.

Remove Intl polyfill
2 participants