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

Bug: Cinny loads 1,837 twemoji on startup #88

Closed
BBaoVanC opened this issue Sep 3, 2021 · 6 comments · Fixed by #259
Closed

Bug: Cinny loads 1,837 twemoji on startup #88

BBaoVanC opened this issue Sep 3, 2021 · 6 comments · Fixed by #259
Labels
type: improvement Functionality that doesn't exist but can be improved
Milestone

Comments

@BBaoVanC
Copy link

BBaoVanC commented Sep 3, 2021

Describe the bug

When Cinny is signed in, it loads 1,837 twemoji on startup, which uses a lot of bandwidth, and causes my browser to lag.

To Reproduce

Steps to reproduce the behavior:

  1. Open Network tab in developer console, and search "twemoji"
  2. Sign into Cinny (or be already signed in)
  3. See a lot of requests

Expected behavior

The twemoji should be loaded when they're sent/received in chat (and/or cached), not all at once on startup.

Screenshots

1,837 requests to twemoji.maxcdn.com, totalling 2.45 MB (1.40 MB compressed)

Desktop (please complete the following information):

  • OS: Linux
  • Browser: Firefox
  • Version: 93
@hifi
Copy link

hifi commented Sep 4, 2021

Additionally all of these need to be locally available from the built bundle and not loaded from an external website. Same for fonts.

@kfiven
Copy link
Collaborator

kfiven commented Sep 4, 2021

These are only loaded from cdn during first sign in, rest of the time we load them from cache.

And I don't think making them locally available will change anything because then we'll have to load them like we load icons now.

What needs to be done is delay the loading so that it doesn't hang loading screen.

@kfiven
Copy link
Collaborator

kfiven commented Sep 4, 2021

or load them when people open emojiboard.

@hifi
Copy link

hifi commented Sep 4, 2021

There are tons of reasons not to use a CDN. First and foremost it makes Cinny depend on a random CDN to even function properly, then packaging Cinny (in any shape or form) is impossible as it will require resources that are not part of the build (think mobile and desktop). It also makes Cinny unsuitable to use in internal communication without internet access however unlikely that scenario is. It also makes self hosting Cinny not really a thing as it's not fully contained anyway.

Loading thousands of emojis to memory even when you only see something like sub-100 at most per day is wasted resources where Cinny certainly seems to aim at being lightweight. It definitely should lazy load them on demand and then let the browser cache do its magic in the future making lazy loading practically instant.

It's fine at this point in time to use a CDN as a first step but it needs to be replaced in the long run.

@vinceTheProgrammer
Copy link

or load them when people open emojiboard.

I don't know anything about web requests, CDNs, or twemoji, but if that would just make it lag when you open the emojiboard instead of on first sign in, then I'm definitely against it. I would much rather it load on startup so that my flow isn't interrupted mid session when I open the emojiboard.

If you're talking lazy loading tho, I think that could work.

@ajbura ajbura added the type: improvement Functionality that doesn't exist but can be improved label Sep 20, 2021
@ajbura ajbura added this to the v1.7.0 milestone Dec 14, 2021
@N-R-K
Copy link

N-R-K commented Jan 13, 2022

I would suggest not lazy loading everything. Since there's a handful of emojies which are often used, pack them into a "frequently used" group and pre-load these. The rest can be lazily loaded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: improvement Functionality that doesn't exist but can be improved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants