feat: pass nonce to color scheme script through app provider#4917
feat: pass nonce to color scheme script through app provider#4917apedroferreira merged 3 commits intomui:masterfrom
Conversation
a88zach commented May 1, 2025
a88zach commented - I've read and followed the contributing guide on how to create great pull requests.
- I've updated the relevant documentation for any new or updated feature.
- I've linked relevant GitHub issue with "Closes Cannot pass nonce to InitColorSchemeScript #4912".
- I've added a visual demonstration in the form of a screenshot or video.
Netlify deploy preview |
apedroferreira left a comment
apedroferreira left a comment There was a problem hiding this comment.
Thanks, looks perfect!
Can you just generate the documentation for the new prop by running pnpm proptypes from the repository root and then pnpm docs:build:api? Then it should be good to merge, let me know if you find any issues.
| window?: Window; | ||
| /** | ||
| * The nonce to be used for inline scripts. | ||
| * @default undefined |
There was a problem hiding this comment.
I think it shouldn't need this @default when it's undefined.
But I might be wrong actually, not 100% sure, it will error on the scripts I mentioned if it's needed.
There was a problem hiding this comment.
I was in the process of generating the docs while you were reviewing. Updated and pushed
You are correct about @default not being necessary when the default is undefined
| expect(screen.getByText('Hello world')).toBeTruthy(); | ||
| }); | ||
| | ||
| test('renders nonce correctly on color scheme script', async () => { |
There was a problem hiding this comment.
Awesome, thanks for even adding a test!
| }); | ||
| | ||
| test('renders nonce correctly on color scheme script', async () => { | ||
| const nonce = Buffer.from(crypto.randomUUID()).toString('base64'); |
There was a problem hiding this comment.
Oh and the Buffer here seems to be making the test:browser tests fail.
Maybe you can just use a static string as it's just a test.
There was a problem hiding this comment.
Updated the test to use btoa. Both test and test:browser are passing now
apedroferreira added type: enhancement apedroferreira left a comment
apedroferreira left a comment There was a problem hiding this comment.
Thank you! Looks great.
3 participants
