Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

AR-2148 Fix repeated rendering of global styles for tooltip-like components #330

Merged
merged 3 commits into from
Mar 10, 2021

Conversation

justinanastos
Copy link
Contributor

@justinanastos justinanastos commented Mar 10, 2021

Resolves issue AR-2148

We are rendering <Global> styles for all instances of AbstractTooltip, of which there can be thousands lying in wait on a single page. I incorrectly believed that these would be rendered just once to the DOM, but I was wrong; they are rendered once per instance rendered. This resulted in thousands of duplicated <style> tags being added.

Release Notes

  • Create an internal component called SingletonComponent (the name is poor and can be changed whenever we want because it's internal) that will take a string identity and only allow a single instance in the DOM regardless of how many times it's rendered. This requires us to use SpaceKitProvider. Tooltips can be used without SpaceKitProvider and will warn the user in the console.
  • Wrap the two <Global> styles with the SingletonComponent to remove all excess DOM <style />s
📦 Published PR as canary version: 8.17.4-canary.330.8834.0

✨ Test out this PR locally via:

npm install @apollo/space-kit@8.17.4-canary.330.8834.0
# or 
yarn add @apollo/space-kit@8.17.4-canary.330.8834.0

@justinanastos justinanastos added the bug fix Increment the patch version when merged label Mar 10, 2021
@justinanastos justinanastos force-pushed the justin/AR-2148/limit-global-renders branch 3 times, most recently from 4493c22 to 57261e0 Compare March 10, 2021 15:58
@justinanastos justinanastos marked this pull request as ready for review March 10, 2021 15:58
@justinanastos justinanastos requested a review from a team March 10, 2021 15:58
@justinanastos justinanastos enabled auto-merge (squash) March 10, 2021 15:58
Copy link
Member

@cheapsteak cheapsteak left a comment

Choose a reason for hiding this comment

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

I incorrectly believed that these would be rendered just once to the DOM

ack, apologies, ditto, I figured since emotion was using content-hash to make classnames it would be able to avoid creating duplicate style tags

🚢

This will ensure that regardless of how many times you render the component
simultaneously, only a single element will exist in the DOM.
@justinanastos justinanastos force-pushed the justin/AR-2148/limit-global-renders branch from 4327f8a to ffeda9f Compare March 10, 2021 16:47
@justinanastos justinanastos merged commit d796351 into main Mar 10, 2021
@justinanastos justinanastos deleted the justin/AR-2148/limit-global-renders branch March 10, 2021 16:53
@apollo-bot2
Copy link
Collaborator

🚀 PR was released in v8.17.4 🚀

@apollo-bot2 apollo-bot2 added the released This issue/pull request has been released. label Mar 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug fix Increment the patch version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants