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

Use await import for lottie-web in LoadingAnimation #611

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

psarando
Copy link
Member

@psarando psarando commented Dec 7, 2024

Builds on my laptop are failing when trying to build SSR pages that use the LoadingAnimation, such as /instantlaunch/[id].

They are not failing in GitHub workflow skaffold builds, or on other dev machines, for now, but maybe this will prevent future build errors.

@psarando psarando added the bug Something isn't working label Dec 7, 2024
@psarando psarando requested a review from ianmcorvidae December 7, 2024 03:13
Copy link
Member

@ianmcorvidae ianmcorvidae left a comment

Choose a reason for hiding this comment

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

Seems good to me. It already was building fine (for me), but lottie-web is pretty big anyway so it's good to keep out of the bundles. As long as the loading animation is still displaying fine, this seems like a pretty clear win.

Builds on my laptop are failing when trying to build SSR pages
that use the LoadingAnimation, such as `/instantlaunch/[id]`.

They are not failing in GitHub workflow skaffold builds,
or on other dev machines, for now,
but maybe this will prevent future build errors.
@psarando psarando force-pushed the dynamic-loading-animation branch from 3ed3123 to de22625 Compare December 9, 2024 21:00
@psarando psarando changed the title Use next/dynamic to import lottie-web in LoadingAnimation Use await import for lottie-web in LoadingAnimation Dec 9, 2024
@psarando
Copy link
Member Author

psarando commented Dec 9, 2024

Seems good to me... As long as the loading animation is still displaying fine, this seems like a pretty clear win.

Oops, helps if I actually test these changes 😊

Turns out I used the wrong kind of dynamic/async import.
It's working now with await import, so I'll go ahead and merge this.

Thanks for the review! 👍

@psarando psarando merged commit ff9b9b9 into cyverse-de:main Dec 9, 2024
6 checks passed
@psarando psarando deleted the dynamic-loading-animation branch December 9, 2024 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants