Fix: Implement lazy load lottie animations#410
Conversation
|
@ahana4banerjee is attempting to deploy a commit to the magic-peach1's projects Team on Vercel. A member of the Team first needs to authorize it. |
👋 Thanks for your PR, @ahana4banerjee!Welcome to Reframe — a browser-based video editor built for everyone 🎬 What happens next
Quick checklist
Useful links
Happy coding! 🎉 |
|
|
Hey @ahana4banerjee! Thanks for the PR 🙏 One issue: this project uses Bun as the package manager, so Fix:
git rm package-lock.json
git add bun.lock package.json # if changed
git commit --amend --no-edit
git push --force-with-lease
|
|
@magic-peach Done! Please Check |
|
@ahana4banerjee please resolve conflicts |
|
Hey @ahana4banerjee! The build is failing with two ESLint errors. Here's exactly what to fix: Error 1 — Your Fix: Move the export function ExportOverlay({ visible, ... }) {
// ALL hooks must come first
useEffect(() => {
// your effect
}, [visible]);
if (!visible) return null; // early return AFTER hooks
return <div>...</div>;
}Error 2 — You have a Fix: Either change the <div
role="button"
tabIndex={0}
onClick={handleClick}
onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') handleClick(); }}
>Fix both and push — CI should pass. 🚀 |
|
Hi @ahana4banerjee 👋 This PR has a merge conflict with git fetch upstream && git merge upstream/main
# resolve any conflicts, then:
git pushOnce conflicts are resolved and CI passes, this will be ready to merge. 🙂 |
|
Hey @ahana4banerjee! The build is failing with two ESLint errors:
Please fix these and push an update! |
|
Hey @ahana4banerjee, this PR has merge conflicts with main. Please merge/rebase main into your branch and resolve the conflicts before we can review it. Run: |
|
Hey @ahana4banerjee! The lazy lottie loading implementation looks great — dynamic import with a mounted check is exactly the right approach. However, this PR has merge conflicts with Since PR #513 (which also modified git fetch origin
git checkout fix/lazy-load-lottie
git rebase origin/main
# resolve conflicts in DownloadResult.tsx (keep both the filename editing AND your lazy lottie loading)
git push --force-with-leaseOnce rebased and CI passes, this will be ready to merge! |
|
Hey @ahana4banerjee! The build check is failing for this PR. Please review the build logs, fix the error, and push an update. Once the build passes and the code looks clean, this will be ready to merge. Let me know if you need help identifying the issue! |
|
Hey @ahana4banerjee! Lazy-loading Lottie animations is a good performance optimization. This PR has several issues:
Please rebase and fix all three CI failures before this can be reviewed. |
|
Hey @ahana4banerjee! This PR has CI check failures (build/lint/typecheck) and merge conflicts with
git fetch origin
git rebase origin/main
# fix any errors, then:
git push --force-with-leaseOnce all checks pass, we'll review and merge! |
|
Hey @ahana4banerjee! This PR had merge conflicts with |
|
Hey @ahana4banerjee! I've resolved the merge conflicts and pushed, but the build/lint/typecheck checks are still failing. Please check the build logs and fix the errors. The lazy-loading approach looks interesting — just needs the build issues resolved! |
9d1bce7 to
5a0b5e8
Compare
|
Hey @ahana4banerjee! The build was failing due to unresolved conflict markers. I've fixed the conflict resolution and pushed again — CI has been re-triggered. Once it passes, we'll review the lazy Lottie loading feature and merge! |
5a0b5e8 to
9246bbe
Compare
|
Fixed again! The |
|
@ahana4banerjee all checks are failing please check |
|
Hey @ahana4banerjee! 👋 We've added a new requirement for all PRs: a screen recording showing your changes working on your local machine must be attached before a PR can be merged. Please add a recording to this PR that shows:
How to record:
Once you have the recording, drag the file directly into a comment on this PR, or paste a Loom link. This is now a hard requirement — see CONTRIBUTING.md for full details. Thanks for contributing to Reframe! 🎬 |
Description
With reference to issue #187, this PR optimizes animation asset loading by replacing static Lottie JSON imports with dynamic imports for on-demand loading.
Previously, the animation JSON files (
spinner.json,success.json, andupload.json) were statically imported, causing them to be included in the initial application bundle even when the related UI components were not rendered immediately.This update introduces lazy loading for animation assets to improve code-splitting behavior and reduce unnecessary eager loading during initial render.
Changes Made
Replaced Static Imports with Dynamic Imports
Removed static imports such as:
and replaced them with lazy-loaded imports using
import()insideuseEffecthooks.Components Updated
ExportOverlay.tsxspinner.jsonFileUpload.tsxupload.jsonDownloadResult.tsxsuccess.jsonPerformance Improvements
On-Demand Animation Loading
Animation JSON files are now fetched only when their respective components mount instead of during the initial application load.
Improved Code Splitting
Verified that Lottie animation assets are now emitted as separate async chunks during production build.
Example verified from production output:
containing:
This confirms the animation assets are no longer bundled into the initial application payload.
UX Considerations
To prevent visible loading flashes:
Testing
Verified:
Tested on:
bun run build)This PR closes issue #187