Skip to content

ThemeProvider: Fix theming with SSR with CSRF safe approach - #1913

Merged
siddharthkp merged 6 commits into
mainfrom
siddharth/safe-theming-ssr
Mar 7, 2022
Merged

ThemeProvider: Fix theming with SSR with CSRF safe approach#1913
siddharthkp merged 6 commits into
mainfrom
siddharth/safe-theming-ssr

Conversation

@siddharthkp

@siddharthkpsiddharthkp commented Mar 1, 2022

Copy link
Copy Markdown
Member

Following up on #1868, the approach I chose relied on injecting an inline script to set a variable. This doesn't work in github/docs-internal because secure applications prevent executing inline scripts injected in the DOM 😇

Inspired by __NEXT_DATA__ from next.js, I've changed the implementation to inject a <script type="application/json"> instead of <script type="text/javascript">.

Tested with docs-internal, it works!

@siddharthkp
siddharthkp requested review from a team and rezrahMarch 1, 2022 14:54
@changeset-bot

changeset-botBot commented Mar 1, 2022

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 49d2970

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
@primer/reactPatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@siddharthkpsiddharthkp self-assigned this Mar 1, 2022
@siddharthkpsiddharthkp added bug Something isn't working react labels Mar 1, 2022
@siddharthkpsiddharthkp changed the title move to application/json approachThemeProvider with SSR: Move to json apporachMar 1, 2022
@siddharthkpsiddharthkp changed the title ThemeProvider with SSR: Move to json apporachFix theming with SSR with CSRF safe approachMar 1, 2022
@github-actions

github-actionsBot commented Mar 1, 2022

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
dist/browser.esm.js64.31 KB (+0.12% 🔺)
dist/browser.umd.js64.67 KB (+0.11% 🔺)

Comment threadsrc/ThemeProvider.tsx Outdated
@siddharthkp
siddharthkp requested a review from pksjceMarch 3, 2022 17:59
Comment threadsrc/ThemeProvider.tsx Outdated

@colebemiscolebemis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! This is a clever trick 😅

@siddharthkp
siddharthkp changed the base branch from main to next-majorMarch 7, 2022 11:58
@siddharthkp
siddharthkp changed the base branch from next-major to mainMarch 7, 2022 11:58
@siddharthkp
siddharthkp merged commit 92a0237 into mainMar 7, 2022
@siddharthkp
siddharthkp deleted the siddharth/safe-theming-ssr branch March 7, 2022 12:25
This was referenced Mar 7, 2022
@siddharthkpsiddharthkp changed the title Fix theming with SSR with CSRF safe approachThemeProvider: Fix theming with SSR with CSRF safe approachJun 3, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugSomething isn't workingreact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@siddharthkp@pksjce@colebemis