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.

Sign up for GitHub

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

Jump to bottom

[DataGrid] Don't evaluate hasEval when disableEval is set #11516

Merged
merged 3 commits into from
Jan 2, 2024
Merged

[DataGrid] Don't evaluate hasEval when disableEval is set #11516

merged 3 commits into from
Jan 2, 2024

Conversation

reihwald
Copy link
Contributor

@reihwald reihwald commented Dec 25, 2023 •
edited

Merry christmas!

Fixes #11465

I had the same issue as #11465 and changed the evaluation of hasEval so that it no longer writes a warning/error in the console in case eval is blocked by the csp.

I also made disableEval appeir in the DataGrid API as per @flaviendelangle but I'm of course open to reverting this in case it's not wanted.

This change also makes it so that the eval check won't be executed if filterMode='server' regardless of the disableEval prop.

@reihwald reihwald changed the title [DataGrid] Don't evaluate hasEval when disableEval is set. [DataGrid] Don't evaluate hasEval when disableEval is set Dec 25, 2023
@mui-bot
Copy link

mui-bot commented Dec 25, 2023 •
edited

Deploy preview: https://deploy-preview-11516--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 0cd6fbe

@reihwald reihwald mentioned this pull request Dec 25, 2023
Closed
@zannager zannager added the component: data grid This is the name of the generic UI component, not the React module! label Dec 26, 2023
@reihwald reihwald force-pushed the only-evaluate-hasEval-when-disableEval-not-set branch from 40adfc8 to cd5e38c Compare December 26, 2023 11:30
@reihwald
Copy link
Contributor Author

Is the argos fail expected? It seems to be failing because of something unrelated to this pr

@flaviendelangle
Copy link
Member

The Argos diff is due to the fact that you are targeting master and our main branch is currently next
So Argos compare its screenshot to next

Unless this PR is a cherry-pick of a PR already applied to next, it should probably target next and we will take care of back-porting it to master

next contains the code for v7 alpha releases and master contains the code for v6 stable releases.

@reihwald reihwald force-pushed the only-evaluate-hasEval-when-disableEval-not-set branch from cd5e38c to d811287 Compare January 2, 2024 14:07
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 2, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 2, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@reihwald reihwald changed the base branch from master to next January 2, 2024 14:08
@reihwald reihwald force-pushed the only-evaluate-hasEval-when-disableEval-not-set branch from d811287 to 6f12f0a Compare January 2, 2024 14:12
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 2, 2024
@reihwald
Copy link
Contributor Author

reihwald commented Jan 2, 2024

Thanks @flaviendelangle. I changed the branch. Can I help with the back-porting?

@flaviendelangle
Copy link
Member

We have a new bot that should create the PR on master once we merge this one 👍
I'll let the grid team make the review

@romgrk romgrk added the needs cherry-pick The PR should be cherry-picked to master after merge label Jan 2, 2024
@romgrk romgrk enabled auto-merge (squash) January 2, 2024 17:43
auto-merge was automatically disabled January 2, 2024 18:22

Head branch was pushed to by a user without write access

@reihwald reihwald force-pushed the only-evaluate-hasEval-when-disableEval-not-set branch from e564a6f to 0cd6fbe Compare January 2, 2024 18:22
@reihwald
Copy link
Contributor Author

reihwald commented Jan 2, 2024

@romgrk I had to retrigger the pipeline because a check failed. Can you please enable auto-merge again?

@romgrk romgrk merged commit 19b9eb5 into mui:next Jan 2, 2024
17 checks passed
@romgrk
Copy link
Contributor

romgrk commented Jan 2, 2024

All good, thanks for the PR!

github-actions bot pushed a commit that referenced this pull request Jan 2, 2024
@reihwald @web-flow
[DataGrid] Don't evaluate hasEval when disableEval is set (#11516)
fbfdafd
@github-actions github-actions bot mentioned this pull request Jan 2, 2024
Merged
@oliviertassinari oliviertassinari added the bug 🐛 Something doesn't work label Jan 3, 2024
@reihwald reihwald deleted the only-evaluate-hasEval-when-disableEval-not-set branch January 8, 2024 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romgrk romgrk romgrk approved these changes

Assignees
No one assigned
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! needs cherry-pick The PR should be cherry-picked to master after merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

hasEval detection leads to CSP warnings and reports
6 participants
@reihwald @mui-bot @flaviendelangle @romgrk @oliviertassinari @zannager