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

Proposal: Offer an option to use lottie light. #90

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

wpp
Copy link

@wpp wpp commented Aug 20, 2019

Hello!

As mentioned by @eyaleizenberg in #79,
the current bundle size is around 250KB. By using lottie_light over lottie-web one can save
around 100KB (minified). The difference between the 2 versions is explained here:

lottie_light has only the svg renderer and doesn't support expressions.

Since react-lottie always uses the SVG renderer (AFAICT) and expressions might not be
required by everyone (my case), it would be nice if we had a way to expose the lottie_light
option.

The PR is just a proposal, since I'm not sure how often expressions are required. But I think
100KB is worth a discussion. Let me know what you think.

wpp added 2 commits August 20, 2019 14:47
AFAICT react-lottie always uses the svg renderer and
(at least in my case) the animations don't have expressions.

By importing the light version of of lottie-web one can save around
100KB (minified).

airbnb/lottie-web#929
@eyaleizenberg
Copy link

First of all, thanks!
Second, maybe 2 separate entry points for light/heavy would be a good idea?

@wpp
Copy link
Author

wpp commented Aug 21, 2019

Yes that sounds good. Problem is I don't know what a good approach would be to get 2 builds.
I don't have a lot of experience with js library maintenance so any pointers are appreciated.

I'd also welcome information regarding the prepublish script. It says:

This is an auto generated file with React CDK

Is that referring to https://github.com/storybook-eol/react-cdk? Which seems to be outdated?

From what I understand, there's actually no way to conditionally import the light vs regular versions at build time. So what I ended up doing for now is wrap the existing component and inject the appropriate lottie version as a prop. The tests had to be adjusted accordingly, since the component is now wrapped.

I'm really uncertain about this approach, so feedback is very welcome.

@robertleeplummerjr
Copy link

robertleeplummerjr commented Sep 4, 2019

I think this is a very nice proposal, and in addition to it, I'd be very interested in an exportable function from the animation. This would be similar to the .toString(...args) method of GPU.js: https://github.com/gpujs/gpu.js#using-kerneltostringargs or the .toFunction() method of Brain.js: https://github.com/BrainJS/brain.js#standalone-function or even the way that gl-wiretap works: https://github.com/robertleeplummerjr/gl-wiretap

Imagine you build your animations, and a single method that exports the whole functionality, with little to no library in it, just the raw svg and functionality that make the animation tick. This could be saved as its own unique react component that is exactly and specifically the required animation, with zero library in it.

It would not be useful in all use cases, but in the use case of, for example (mine), a loading spinner, where we've included the whole of lottie just to have a simple and smooth animation, it would make complete sense.

@mjrussell
Copy link

This would be really helpful as lottie also does not support a CSP without unsafe-eval (see airbnb/lottie-web#323). What is left to get this PR merged?

@rubek-joshi
Copy link

Why hasn't this PR been merged? I would love to reduce the bundle size

@gajraj-gan
Copy link

+1

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.

6 participants