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.

Sign up for GitHub

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

Jump to bottom

YoutubeAtom refactors - pass index, extract components and event emitters #12405

Merged
merged 5 commits into from
Sep 19, 2024
Merged

YoutubeAtom refactors - pass index, extract components and event emitters #12405

merged 5 commits into from
Sep 19, 2024
+328 −266

Conversation

arelra
Copy link
Member

@arelra arelra commented Sep 18, 2024 •
edited
Loading

What does this change?

A collection of small refactors to the YoutubeAtom.

  1. Replace the module level index within YoutubeAtom with an index passed from ancestor components that map over YoutubeAtom. This was initially put in place here to remove usage of elementId which is not stable across rendering requests and would therefore cause cache instability by invalidating the eTag on every render.
    However a module level index is not idiomatic React and we can replace this by passing an index from any ancestors that generate YoutubeAtoms.
    The atom id and index are stable across requests. The index will only change if the page structure changes which is expected.
    774a2fc

  2. Extract YouTubeExpiredOverlay component
    77aa0ac

  3. Extract event emitters into their own module
    e2904c5

  4. Add data attributes for the atomId, videoId and uniqueId to aid inspection
    bf067d1

  5. Update tests to reflect above changes

Why?

Simplify the code and make it easier to understand

@arelra arelra requested a review from a team as a code owner September 18, 2024 12:59
@arelra arelra added the run_chromatic Runs chromatic when label is applied label Sep 18, 2024
@github-actions github-actions bot added dotcom-rendering and removed run_chromatic Runs chromatic when label is applied labels Sep 18, 2024
@github-actions GitHub Actions
Copy link

Size Change: +824 B (+0.09%)

Total Size: 939 kB

Filename Size Change
dotcom-rendering/dist/4428.client.web.********************.js 0 B -12.6 kB (removed) 🏆
dotcom-rendering/dist/8883.client.web.********************.js 0 B -3.63 kB (removed) 🏆
dotcom-rendering/dist/555.client.web.********************.js 4.42 kB +4.42 kB (new file) 🆕
dotcom-rendering/dist/636.client.web.********************.js 12.6 kB +12.6 kB (new file) 🆕
ℹ️ View Unchanged
Filename Size Change
dotcom-rendering/dist/1000.client.web.********************.js 999 B 0 B
dotcom-rendering/dist/1026.client.web.********************.js 784 B 0 B
dotcom-rendering/dist/1065.client.web.********************.js 3.83 kB 0 B
dotcom-rendering/dist/1068.client.web.********************.js 3.97 kB 0 B
dotcom-rendering/dist/1090.client.web.********************.js 752 B 0 B
dotcom-rendering/dist/1391.client.web.********************.js 725 B 0 B
dotcom-rendering/dist/1476.client.web.********************.js 784 B 0 B
dotcom-rendering/dist/1642.client.web.********************.js 2.89 kB 0 B
dotcom-rendering/dist/1667.client.web.********************.js 918 B 0 B
dotcom-rendering/dist/1940.client.web.********************.js 507 B 0 B
dotcom-rendering/dist/2310.client.web.********************.js 880 B 0 B
dotcom-rendering/dist/2328.client.web.********************.js 6.05 kB 0 B
dotcom-rendering/dist/2404.client.web.********************.js 3.34 kB 0 B
dotcom-rendering/dist/2477.client.web.********************.js 8.85 kB 0 B
dotcom-rendering/dist/2490.client.web.********************.js 5.12 kB 0 B
dotcom-rendering/dist/250.client.web.********************.js 2.06 kB 0 B
dotcom-rendering/dist/267.client.web.********************.js 917 B 0 B
dotcom-rendering/dist/2734.client.web.********************.js 4.05 kB 0 B
dotcom-rendering/dist/2750.client.web.********************.js 438 B 0 B
dotcom-rendering/dist/2756.client.web.********************.js 2.97 kB 0 B
dotcom-rendering/dist/281.client.web.********************.js 642 B 0 B
dotcom-rendering/dist/2848.client.web.********************.js 618 B 0 B
dotcom-rendering/dist/3109.client.web.********************.js 803 B 0 B
dotcom-rendering/dist/3270.client.web.********************.js 961 B 0 B
dotcom-rendering/dist/3304.client.web.********************.js 853 B 0 B
dotcom-rendering/dist/3306.client.web.********************.js 5.8 kB 0 B
dotcom-rendering/dist/3521.client.web.********************.js 4.41 kB 0 B
dotcom-rendering/dist/3751.client.web.********************.js 3.03 kB 0 B
dotcom-rendering/dist/3769.client.web.********************.js 999 B 0 B
dotcom-rendering/dist/3886.client.web.********************.js 3.78 kB 0 B
dotcom-rendering/dist/3938.client.web.********************.js 4.9 kB 0 B
dotcom-rendering/dist/4282.client.web.********************.js 685 B 0 B
dotcom-rendering/dist/4319.client.web.********************.js 5.62 kB 0 B
dotcom-rendering/dist/435.client.web.********************.js 3.85 kB 0 B
dotcom-rendering/dist/4359.client.web.********************.js 22.7 kB 0 B
dotcom-rendering/dist/4501.client.web.********************.js 4.29 kB 0 B
dotcom-rendering/dist/4628.client.web.********************.js 654 B 0 B
dotcom-rendering/dist/4725.client.web.********************.js 4.28 kB 0 B
dotcom-rendering/dist/4941.client.web.********************.js 890 B 0 B
dotcom-rendering/dist/52.client.web.********************.js 9.75 kB -1 B (-0.01%)
dotcom-rendering/dist/5315.client.web.********************.js 4.91 kB 0 B
dotcom-rendering/dist/5373.client.web.********************.js 2.67 kB 0 B
dotcom-rendering/dist/5392.client.web.********************.js 3.4 kB 0 B
dotcom-rendering/dist/5414.client.web.********************.js 3.66 kB 0 B
dotcom-rendering/dist/5459.client.web.********************.js 2.52 kB 0 B
dotcom-rendering/dist/5546.client.web.********************.js 3.77 kB 0 B
dotcom-rendering/dist/5565.client.web.********************.js 2.69 kB 0 B
dotcom-rendering/dist/5571.client.web.********************.js 13.9 kB +11 B (+0.08%)
dotcom-rendering/dist/5658.client.web.********************.js 750 B 0 B
dotcom-rendering/dist/5757.client.web.********************.js 931 B 0 B
dotcom-rendering/dist/5847.client.web.********************.js 2.42 kB 0 B
dotcom-rendering/dist/5880.client.web.********************.js 828 B 0 B
dotcom-rendering/dist/6044.client.web.********************.js 726 B 0 B
dotcom-rendering/dist/6050.client.web.********************.js 2.93 kB 0 B
dotcom-rendering/dist/6071.client.web.********************.js 577 B 0 B
dotcom-rendering/dist/6135.client.web.********************.js 779 B 0 B
dotcom-rendering/dist/6153.client.web.********************.js 2.99 kB 0 B
dotcom-rendering/dist/6255.client.web.********************.js 18.9 kB 0 B
dotcom-rendering/dist/6309.client.web.********************.js 4.56 kB 0 B
dotcom-rendering/dist/6429.client.web.********************.js 3.42 kB 0 B
dotcom-rendering/dist/6444.client.web.********************.js 4.03 kB 0 B
dotcom-rendering/dist/6449.client.web.********************.js 9.95 kB 0 B
dotcom-rendering/dist/6505.client.web.********************.js 1 kB 0 B
dotcom-rendering/dist/6598.client.web.********************.js 780 B 0 B
dotcom-rendering/dist/6620.client.web.********************.js 3.72 kB 0 B
dotcom-rendering/dist/6638.client.web.********************.js 907 B 0 B
dotcom-rendering/dist/6639.client.web.********************.js 4.67 kB 0 B
dotcom-rendering/dist/6659.client.web.********************.js 1.84 kB 0 B
dotcom-rendering/dist/678.client.web.********************.js 804 B 0 B
dotcom-rendering/dist/6795.client.web.********************.js 523 B 0 B
dotcom-rendering/dist/6989.client.web.********************.js 528 B 0 B
dotcom-rendering/dist/7116.client.web.********************.js 23 kB 0 B
dotcom-rendering/dist/7265.client.web.********************.js 3.23 kB 0 B
dotcom-rendering/dist/7333.client.web.********************.js 2.25 kB 0 B
dotcom-rendering/dist/7470.client.web.********************.js 3.7 kB 0 B
dotcom-rendering/dist/7517.client.web.********************.js 4.07 kB 0 B
dotcom-rendering/dist/7521.client.web.********************.js 4.88 kB 0 B
dotcom-rendering/dist/7560.client.web.********************.js 3.34 kB 0 B
dotcom-rendering/dist/7564.client.web.********************.js 6.29 kB 0 B
dotcom-rendering/dist/7622.client.web.********************.js 155 B 0 B
dotcom-rendering/dist/7673.client.web.********************.js 3.21 kB 0 B
dotcom-rendering/dist/7691.client.web.********************.js 853 B 0 B
dotcom-rendering/dist/7741.client.web.********************.js 20.2 kB 0 B
dotcom-rendering/dist/7754.client.web.********************.js 2.68 kB 0 B
dotcom-rendering/dist/7899.client.web.********************.js 7.63 kB 0 B
dotcom-rendering/dist/8064.client.web.********************.js 3.58 kB -2 B (-0.06%)
dotcom-rendering/dist/8096.client.web.********************.js 3.06 kB 0 B
dotcom-rendering/dist/8156.client.web.********************.js 4.51 kB 0 B
dotcom-rendering/dist/83.client.web.********************.js 750 B 0 B
dotcom-rendering/dist/8389.client.web.********************.js 4.11 kB 0 B
dotcom-rendering/dist/8458.client.web.********************.js 4.66 kB 0 B
dotcom-rendering/dist/8504.client.web.********************.js 827 B 0 B
dotcom-rendering/dist/8536.client.web.********************.js 595 B 0 B
dotcom-rendering/dist/8573.client.web.********************.js 3.43 kB 0 B
dotcom-rendering/dist/8626.client.web.********************.js 890 B 0 B
dotcom-rendering/dist/8697.client.web.********************.js 956 B 0 B
dotcom-rendering/dist/8795.client.web.********************.js 3.83 kB 0 B
dotcom-rendering/dist/8833.client.web.********************.js 829 B 0 B
dotcom-rendering/dist/9103.client.web.********************.js 5.93 kB 0 B
dotcom-rendering/dist/9166.client.web.********************.js 3.38 kB 0 B
dotcom-rendering/dist/9184.client.web.********************.js 493 B 0 B
dotcom-rendering/dist/9241.client.web.********************.js 2.94 kB 0 B
dotcom-rendering/dist/9493.client.web.********************.js 785 B 0 B
dotcom-rendering/dist/9512.client.web.********************.js 3.01 kB 0 B
dotcom-rendering/dist/9557.client.web.********************.js 921 B 0 B
dotcom-rendering/dist/9569.client.web.********************.js 44.2 kB -2 B (0%)
dotcom-rendering/dist/9721.client.web.********************.js 717 B 0 B
dotcom-rendering/dist/9835.client.web.********************.js 647 B 0 B
dotcom-rendering/dist/9899.client.web.********************.js 669 B 0 B
dotcom-rendering/dist/Accessibility-importable.client.web.********************.js 7.92 kB 0 B
dotcom-rendering/dist/AdBlockAsk-importable.client.web.********************.js 2.85 kB 0 B
dotcom-rendering/dist/AdPortals-importable.client.web.********************.js 3.93 kB 0 B
dotcom-rendering/dist/AlreadyVisited-importable.client.web.********************.js 421 B 0 B
dotcom-rendering/dist/AppsEpic-importable.client.web.********************.js 3.57 kB -1 B (-0.03%)
dotcom-rendering/dist/AppsFooter-importable.client.web.********************.js 3.65 kB 0 B
dotcom-rendering/dist/AppsLightboxImage-importable.client.web.********************.js 3.07 kB -1 B (-0.03%)
dotcom-rendering/dist/AppsLightboxImageStore-importable.client.web.********************.js 2.5 kB 0 B
dotcom-rendering/dist/AudioAtomWrapper-importable.client.web.********************.js 3.58 kB 0 B
dotcom-rendering/dist/AustralianTerritorySwitcher-importable.client.web.********************.js 4.89 kB 0 B
dotcom-rendering/dist/Branding-importable.client.web.********************.js 3.06 kB 0 B
dotcom-rendering/dist/braze-web-sdk-core.client.web.********************.js 37.2 kB 0 B
dotcom-rendering/dist/BrazeMessaging-importable.client.web.********************.js 1.96 kB 0 B
dotcom-rendering/dist/CalloutBlockComponent-importable.client.web.********************.js 6.71 kB +2 B (+0.03%)
dotcom-rendering/dist/CalloutEmbedBlockComponent-importable.client.web.********************.js 7.48 kB -2 B (-0.03%)
dotcom-rendering/dist/CardCommentCount-importable.client.web.********************.js 3.48 kB 0 B
dotcom-rendering/dist/Carousel-importable.client.web.********************.js 7.24 kB +6 B (+0.08%)
dotcom-rendering/dist/CarouselForNewsletters-importable.client.web.********************.js 4.5 kB 0 B
dotcom-rendering/dist/ChartAtom-importable.client.web.********************.js 541 B 0 B
dotcom-rendering/dist/CommentCount-importable.client.web.********************.js 3.3 kB 0 B
dotcom-rendering/dist/DiscussionApps-importable.client.web.********************.js 1.91 kB 0 B
dotcom-rendering/dist/DiscussionMeta-importable.client.web.********************.js 1.22 kB 0 B
dotcom-rendering/dist/DiscussionWeb-importable.client.web.********************.js 1.74 kB 0 B
dotcom-rendering/dist/DocumentBlockComponent-importable.client.web.********************.js 3.63 kB 0 B
dotcom-rendering/dist/Dropdown-importable.client.web.********************.js 1.73 kB 0 B
dotcom-rendering/dist/EditionSwitcherBanner-importable.client.web.********************.js 5.96 kB 0 B
dotcom-rendering/dist/EmbedBlockComponent-importable.client.web.********************.js 4.26 kB 0 B
dotcom-rendering/dist/EnhancePinnedPost-importable.client.web.********************.js 2.02 kB 0 B
dotcom-rendering/dist/FetchOnwardsData-importable.client.web.********************.js 2.45 kB 0 B
dotcom-rendering/dist/FilterKeyEventsToggle-importable.client.web.********************.js 968 B 0 B
dotcom-rendering/dist/FocusStyles-importable.client.web.********************.js 619 B 0 B
dotcom-rendering/dist/FollowWrapper-importable.client.web.********************.js 763 B 0 B
dotcom-rendering/dist/FooterLabel-importable.client.web.********************.js 347 B 0 B
dotcom-rendering/dist/FooterReaderRevenueLinks-importable.client.web.********************.js 3.74 kB 0 B
dotcom-rendering/dist/frameworks.client.web.********************.js 20.9 kB 0 B
dotcom-rendering/dist/FrontSubNav-importable.client.web.********************.js 5.45 kB 0 B
dotcom-rendering/dist/GetCricketScoreboard-importable.client.web.********************.js 3.81 kB 0 B
dotcom-rendering/dist/GetMatchNav-importable.client.web.********************.js 10.4 kB 0 B
dotcom-rendering/dist/GetMatchStats-importable.client.web.********************.js 7.88 kB 0 B
dotcom-rendering/dist/GetMatchTabs-importable.client.web.********************.js 2.71 kB 0 B
dotcom-rendering/dist/guardian-braze-components-banner.client.web.********************.js 17.3 kB 0 B
dotcom-rendering/dist/guardian-braze-components-end-of-article.client.web.********************.js 11.6 kB 0 B
dotcom-rendering/dist/GuideAtomWrapper-importable.client.web.********************.js 3.01 kB 0 B
dotcom-rendering/dist/HighlightsContainer-importable.client.web.********************.js 4.99 kB 0 B
dotcom-rendering/dist/index.client.web.********************.js 45.6 kB -9 B (-0.02%)
dotcom-rendering/dist/InstagramBlockComponent-importable.client.web.********************.js 3.28 kB 0 B
dotcom-rendering/dist/InteractiveAtomMessenger-importable.client.web.********************.js 853 B 0 B
dotcom-rendering/dist/InteractiveBlockComponent-importable.client.web.********************.js 6.33 kB 0 B
dotcom-rendering/dist/InteractiveContentsBlockComponent-importable.client.web.********************.js 4.71 kB 0 B
dotcom-rendering/dist/KeyEventsCarousel-importable.client.web.********************.js 4.75 kB 0 B
dotcom-rendering/dist/KnowledgeQuizAtom-importable.client.web.********************.js 3.52 kB +2 B (+0.06%)
dotcom-rendering/dist/LatestLinks-importable.client.web.********************.js 4.88 kB 0 B
dotcom-rendering/dist/LightboxHash-importable.client.web.********************.js 435 B 0 B
dotcom-rendering/dist/LightboxLayout-importable.client.web.********************.js 8.2 kB 0 B
dotcom-rendering/dist/LiveBlogEpic-importable.client.web.********************.js 3.79 kB 0 B
dotcom-rendering/dist/LiveblogNotifications-importable.client.web.********************.js 3.15 kB +1 B (+0.03%)
dotcom-rendering/dist/Liveness-importable.client.web.********************.js 5.52 kB 0 B
dotcom-rendering/dist/ManyNewsletterSignUp-importable.client.web.********************.js 7.84 kB 0 B
dotcom-rendering/dist/MapEmbedBlockComponent-importable.client.web.********************.js 5.44 kB 0 B
dotcom-rendering/dist/Metrics-importable.client.web.********************.js 2.68 kB 0 B
dotcom-rendering/dist/MostViewedFooter-importable.client.web.********************.js 5.71 kB 0 B
dotcom-rendering/dist/MostViewedFooterData-importable.client.web.********************.js 8.34 kB 0 B
dotcom-rendering/dist/MostViewedRightWithAd-importable.client.web.********************.js 4.71 kB 0 B
dotcom-rendering/dist/OnwardsUpper-importable.client.web.********************.js 5.84 kB 0 B
dotcom-rendering/dist/PersonalityQuizAtom-importable.client.web.********************.js 3.66 kB -1 B (-0.03%)
dotcom-rendering/dist/ProfileAtom-importable.client.web.********************.js 2.8 kB 0 B
dotcom-rendering/dist/ProfileAtomWrapper-importable.client.web.********************.js 3.03 kB 0 B
dotcom-rendering/dist/PulsingDot-importable.client.web.********************.js 750 B 0 B
dotcom-rendering/dist/QandaAtom-importable.client.web.********************.js 2.79 kB 0 B
dotcom-rendering/dist/ReaderRevenueDev-importable.client.web.********************.js 468 B 0 B
dotcom-rendering/dist/readerRevenueDevUtils.client.web.********************.js 1.97 kB 0 B
dotcom-rendering/dist/RelativeTime-importable.client.web.********************.js 2.53 kB 0 B
dotcom-rendering/dist/RichLinkComponent-importable.client.web.********************.js 5.21 kB 0 B
dotcom-rendering/dist/SecureSignup-importable.client.web.********************.js 5.11 kB 0 B
dotcom-rendering/dist/SendTargetingParams-importable.client.web.********************.js 2.19 kB -1 B (-0.05%)
dotcom-rendering/dist/sentry.client.web.********************.js 794 B 0 B
dotcom-rendering/dist/SetABTests-importable.client.web.********************.js 3.69 kB 0 B
dotcom-rendering/dist/SetAdTargeting-importable.client.web.********************.js 484 B 0 B
dotcom-rendering/dist/ShareButton-importable.client.web.********************.js 1.53 kB 0 B
dotcom-rendering/dist/shimport.client.web.********************.js 2.8 kB 0 B
dotcom-rendering/dist/ShowHideContainers-importable.client.web.********************.js 647 B 0 B
dotcom-rendering/dist/ShowMore-importable.client.web.********************.js 5.54 kB 0 B
dotcom-rendering/dist/SignInGateMain.client.web.********************.js 4.53 kB 0 B
dotcom-rendering/dist/SignInGateMainCheckoutComplete.client.web.********************.js 3.93 kB 0 B
dotcom-rendering/dist/SignInGateSelector-importable.client.web.********************.js 3.15 kB 0 B
dotcom-rendering/dist/SlotBodyEnd-importable.client.web.********************.js 3.48 kB 0 B
dotcom-rendering/dist/SpotifyBlockComponent-importable.client.web.********************.js 5.3 kB 0 B
dotcom-rendering/dist/StickyBottomBanner-importable.client.web.********************.js 4.2 kB 0 B
dotcom-rendering/dist/StickyLiveblogAskWrapper-importable.client.web.********************.js 7.6 kB 0 B
dotcom-rendering/dist/SubNav-importable.client.web.********************.js 3.41 kB 0 B
dotcom-rendering/dist/TableOfContents-importable.client.web.********************.js 3.19 kB 0 B
dotcom-rendering/dist/TimelineAtom-importable.client.web.********************.js 3.23 kB 0 B
dotcom-rendering/dist/Titlepiece-importable.client.web.********************.js 14.7 kB 0 B
dotcom-rendering/dist/TopBar-importable.client.web.********************.js 8.26 kB 0 B
dotcom-rendering/dist/TopBarSupport-importable.client.web.********************.js 2.75 kB 0 B
dotcom-rendering/dist/TweetBlockComponent-importable.client.web.********************.js 1.13 kB 0 B
dotcom-rendering/dist/UnsafeEmbedBlockComponent-importable.client.web.********************.js 3.29 kB 0 B
dotcom-rendering/dist/VideoFacebookBlockComponent-importable.client.web.********************.js 5.46 kB 0 B
dotcom-rendering/dist/VineBlockComponent-importable.client.web.********************.js 3.12 kB 0 B
dotcom-rendering/dist/WeatherWrapper-importable.client.web.********************.js 7.06 kB 0 B
dotcom-rendering/dist/YoutubeBlockComponent-importable.client.web.********************.js 3.94 kB 0 B

compressed-size-action

domlander
domlander approved these changes Sep 18, 2024
View reviewed changes
Copy link
Contributor

@domlander domlander left a comment

Choose a reason for hiding this comment

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

Very nice 👍

@arelra arelra merged commit a2e1af3 into main Sep 19, 2024
35 checks passed
@arelra arelra deleted the ravi/youtube-atom-refactors branch September 19, 2024 09:01
@prout-bot
Copy link

Seen on PROD (merged by @arelra 8 minutes and 41 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@domlander domlander domlander approved these changes

Assignees
No one assigned
Labels
dotcom-rendering Seen-on-PROD
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@arelra @prout-bot @domlander