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 separate emotion 'cache' for pinboard (to avoid it fighting/racing the host application) #293

Merged
merged 3 commits into from
Aug 8, 2023

Conversation

twrichards
Copy link
Collaborator

@twrichards twrichards commented Aug 7, 2023

https://trello.com/c/lDPWxVEu/1627-investigate-styling-bug-in-composer-elements

Co-authored-by: @rebecca-thompson

@rebecca-thompson observed (when testing https://github.com/guardian/flexible-content/pull/4374) that certain styles (defined in prosemirror-elements for Cartoon Element) were not being applied, yet the emotion style block in <head> was empty.

Turns out pinboard (which has all its styles actually defined within various shadow DOMs) was winning in the race/fight over the 'default emotion cache' (see https://emotion.sh/docs/@emotion/cache).

See emotion-js/emotion#2209, so bumped to latest and this looks to have fixed.

Whilst troubleshooting we made Pinboard use its own 'cache' (via https://emotion.sh/docs/cache-provider) which we've left in as it provides nicer separation (even though all meaningful Pinboard styles are actually in the various Shadow DOMs).

Lastly (to aid future debugging etc.) we've made sure all css class names generated by emotion include meaningful names, e.g.
image

@twrichards twrichards marked this pull request as ready for review August 7, 2023 16:31
@twrichards twrichards requested a review from a team as a code owner August 7, 2023 16:31
@@ -16,7 +16,7 @@
},
"dependencies": {
"@apollo/client": "^3.6.5",
"@emotion/react": "^11.1.4",
"@emotion/react": "11.11.1",
Copy link

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed, it's been my preference for a good while!

Copy link

Choose a reason for hiding this comment

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

FYI – this script can use your package.json and yarn.lock to get the currently exact versions of all dependencies. There’s a similar, JS-only one in dotcom-rendering, that ensures this recommendation is followed.

Resolved dependencies in client/package.json
  dependencies: {
    "@apollo/client": "3.6.5"
    "@emotion/react": "11.1.4"
    "@guardian/source-foundations": "4.1.0"
    "@guardian/source-react-components": "4.4.0"
    "@guardian/user-telemetry-client": "1.1.0"
    "@sentry/react": "6.18.1"
    "@webscopeio/react-textarea-autocomplete": "4.9.2"
    "apollo-link-debounce": "3.0.0"
    "aws-appsync-auth-link": "3.0.7"
    "aws-appsync-subscription-link": "3.0.9"
    "date-fns": "2.19.0"
    "graphql": "15.4.0"
    "preact": "10.15.1"
    "react-draggable": "4.4.5"
    "react-joyride": "2.5.3"
    "react-shadow": "19.0.2"
  },

@twrichards twrichards enabled auto-merge August 8, 2023 08:19
@twrichards twrichards force-pushed the fix-conflicting-emotion-cache branch from 406a8ac to b4f1a03 Compare August 8, 2023 08:19
@twrichards twrichards merged commit 5ad5fa5 into main Aug 8, 2023
@twrichards twrichards deleted the fix-conflicting-emotion-cache branch August 8, 2023 08:23
@prout-bot
Copy link
Collaborator

Seen on PROD (merged by @twrichards 4 minutes and 24 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants