Skip to content

feat: pass nonce to color scheme script through app provider - #4917

Merged
apedroferreira merged 3 commits into
mui:masterfrom
a88zach:issue-4912
May 1, 2025
Merged

feat: pass nonce to color scheme script through app provider#4917
apedroferreira merged 3 commits into
mui:masterfrom
a88zach:issue-4912

Conversation

@a88zach

@a88zach a88zach commented May 1, 2025

Copy link
Copy Markdown
Contributor

Screenshot 2025-05-01 at 1 10 47 PM

@mui-bot

mui-bot commented May 1, 2025

Copy link
Copy Markdown

Netlify deploy preview

https://deploy-preview-4917--mui-toolpad-docs.netlify.app/

Generated by 🚫 dangerJS against 5fb3597

@apedroferreira apedroferreira left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

@apedroferreira apedroferreira May 1, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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 () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Awesome, thanks for even adding a test!

});

test('renders nonce correctly on color scheme script', async () => {
const nonce = Buffer.from(crypto.randomUUID()).toString('base64');

@apedroferreira apedroferreira May 1, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated the test to use btoa. Both test and test:browser are passing now

@apedroferreira apedroferreira self-assigned this May 1, 2025
@apedroferreira apedroferreira added type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. scope: toolpad-core Abbreviated to "core" labels May 1, 2025

@apedroferreira apedroferreira left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you! Looks great.

@apedroferreira
apedroferreira merged commit 86d4fa8 into mui:master May 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: toolpad-core Abbreviated to "core" type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot pass nonce to InitColorSchemeScript

3 participants