Lazy load ViewShot #5517
Merged
Lazy load ViewShot #5517
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
arcalinea
temporarily deployed
to
hailey/lazy-view-shot - social-app PR #5517
Render
Destroyed
|
gaearon
reviewed
Sep 27, 2024
| @@ -33,12 +37,12 @@ export const PlaceholderCanvas = React.forwardRef<PlaceholderCanvasRef, {}>( | |||
|
|
|||
| React.useImperativeHandle(ref, () => ({ | |||
| // @ts-ignore this library doesn't have types | |||
| capture: viewshotRef.current.capture, | |||
| capture: viewshotRef.current?.capture, | |||
There was a problem hiding this comment.
are we ok with this method being null in some cases? should we instead wrap to make it a noop?
There was a problem hiding this comment.
i tweaked it like this so it will show up as possibly being undefined 3b220f6 (#5517)
im going to see if i can just get rid of that ts-ignore though, i think there actually is a type for this
There was a problem hiding this comment.
okay yea, this feels nicer 1006d70 (#5517) (plus this to only import the type, oops 18462ad)
arcalinea
temporarily deployed
to
hailey/lazy-view-shot - social-app PR #5517
Render
Destroyed
arcalinea
temporarily deployed
to
hailey/lazy-view-shot - social-app PR #5517
Render
Destroyed
haileyok
force-pushed
the
hailey/lazy-view-shot
branch
from
a14804d to
1006d70
Compare
arcalinea
temporarily deployed
to
hailey/lazy-view-shot - social-app PR #5517
Render
Destroyed
arcalinea
temporarily deployed
to
hailey/lazy-view-shot - social-app PR #5517
Render
Destroyed
gaearon
approved these changes
Sep 27, 2024
estrattonbailey
added a commit
that referenced
this pull request
Sep 30, 2024
* origin/main: Use Inter variable font (#5540) Fix name in `feature_request.yml` (#5542) Fix sticky offset, gear color (#5537) Move email test to root tests dir (#5527) revamp issue templates, add new arch template (#5532) use PressableScale for animation (#5541) Rework native autocomplete (#5521) Pinned posts (#5055) Remove Segment (#5518) Fix alignment of cancel button on search (#5520) [Share Extension] Support on Android for sharing videos to app (#5466) Ignore bogus onScroll values (#5499) add podcasts to spotify embeds (#5514) Tweak font size of "Write your reply" (#5513) Lazy load ViewShot (#5517) [Share Extension] Support images/movies from other apps like iMessage (#5515)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why
Now that we are able to, we should lazy-load
ViewShot, since it saves us almost 80kbHow
Just using
React.lazyandReact.Suspense. Extremely basic for the QR code dialog, we just useReact.Suspenseto show the spinner untilview-shothas loaded.For signup, we won't actually use
React.Suspense, because we don't want the user to get blocked from signing up for any reason. However, we will try to make sure it's ready to go by the time the user gets to that screen.import()forview-shotTest Plan
Go through an account creation, you should still get one of the "new" default avatars. Also verify that QR codes still work.