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

Simplify loading emoji data #1790

Merged
merged 1 commit into from
Nov 1, 2023
Merged

Simplify loading emoji data #1790

merged 1 commit into from
Nov 1, 2023

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Nov 1, 2023

The way we were loading emoji data was split into several parts:

  1. We had a PROD-only file bskyweb/static/emojis.2023.json with the actual emoji data.
  2. We had a script /static/js/emoji-mart-data.js that loads that file and sets it to window.emojiMartData.
  3. We had a line of code in polyfills that injects that script.
  4. Finally, the emoji picker reads window.emojiMartData at render time.

This "worked" but in a confusing way:

  • The line triggering the script load was nested in an incorrect if (!Intl.Segmenter) condition, so it wasn't getting triggered in Chrome.
  • However, in production, that script was injected regardless into <head> because our scripts.html helper does that at build time.
  • Still, this didn't cause problems in local development because when you pass data={undefined} to EmojiMart, it just fetches the data from jsdelivr CDN (which it did in local development).
  • Also, reading from window.emojiMartData during render is effectively a race condition since you don't know whether it's loaded by now or not.
  • Finally, even though this worked, we were loading emojis early every time.

This changes the code to use an async function as data prop, per this example. Instead of PROD-only bskyweb/static/, I opted to make it a dynamic JS chunk which makes it easy to load it on demand without messing with the static file pipeline.

Test plan

Observe emoji data loading on demand (when I open the picker).
Observe that it loads from our domain (not CDN).

Screenshot 2023-11-01 at 17 37 29

@gaearon gaearon requested a review from pfrazee November 1, 2023 17:46
Copy link
Collaborator

@pfrazee pfrazee left a comment

Choose a reason for hiding this comment

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

Nice

@gaearon gaearon merged commit f9944b5 into main Nov 1, 2023
4 checks passed
@gaearon gaearon deleted the emoji-load branch November 1, 2023 17:49
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