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

https://tools-public.mui.com/prod/pages/1t353h04 broken since Toolpad v0.1.20 #131

Closed
oliviertassinari opened this issue Aug 30, 2023 · 2 comments · Fixed by mui/toolpad#2586
Assignees
Labels
bug 🐛 Something doesn't work regression A bug, but worse

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 30, 2023

Context 🔦

It works in v0.1.19.

It crashes in v0.1.20 with:

Screenshot 2023-08-30 at 14 49 56

Noticed this in https://www.notion.so/GitHub-community-issues-PRs-Tier-1-12a84fdf50e44595afc55343dac00fca?d=18038d4bbdbb40bc8a1ca714803aef13&pvs=4#c59aa53a2054400b98613dfb84b4cb03

Maybe mui/toolpad#1991 is the origin.

Your environment 🌎

npx @mui/envinfo
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.
@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work regression A bug, but worse labels Aug 30, 2023
@oliviertassinari oliviertassinari changed the title https://tools-public.mui.com/prod/pages/1t353h04 broken since Toolpad v0.0.20 https://tools-public.mui.com/prod/pages/1t353h04 broken since Toolpad v0.1.20 Aug 30, 2023
@Janpot
Copy link
Member

Janpot commented Aug 30, 2023

We started being more strict about which browser APIs can be used. I think we can whitelist Intl:

diff --git a/packages/toolpad-core/src/jsBrowserRuntime.tsx b/packages/toolpad-core/src/jsBrowserRuntime.tsx
index 6d3611553..6baae4ab3 100644
--- a/packages/toolpad-core/src/jsBrowserRuntime.tsx
+++ b/packages/toolpad-core/src/jsBrowserRuntime.tsx
@@ -20,7 +20,7 @@ function createBrowserRuntime(): JsRuntime {
       (() => {
         // See https://tc39.es/ecma262/multipage/global-object.html#sec-global-object
         const ecmaGlobals = new Set([ 'globalThis', 'Infinity', 'NaN', 'undefined', 'eval', 'isFinite', 'isNaN', 'parseFloat', 'parseInt', 'decodeURI', 'decodeURIComponent', 'encodeURI', 'encodeURIComponent', 'AggregateError', 'Array', 'ArrayBuffer', 'BigInt', 'BigInt64Array', 'BigUint64Array', 'Boolean', 'DataView', 'Date', 'Error', 'EvalError', 'FinalizationRegistry', 'Float32Array', 'Float64Array', 'Function', 'Int8Array', 'Int16Array', 'Int32Array', 'Map', 'Number', 'Object', 'Promise', 'Proxy', 'RangeError', 'ReferenceError', 'RegExp', 'Set', 'SharedArrayBuffer', 'String', 'Symbol', 'SyntaxError', 'TypeError', 'Uint8Array', 'Uint8ClampedArray', 'Uint16Array', 'Uint32Array', 'URIError', 'WeakMap', 'WeakRef', 'WeakSet', 'Atomics', 'JSON', 'Math', 'Reflect' ]);
-        const allowedDomGlobals = new Set([ 'setTimeout', 'console', 'URL', 'URLSearchParams' ])
+        const allowedDomGlobals = new Set([ 'setTimeout', 'console', 'URL', 'URLSearchParams', 'Intl' ])
 
         // NOTE: This is by no means intended to be a secure way to hide DOM globals 
         const globalThis = new Proxy(window.__SCOPE, {

@oliviertassinari
Copy link
Member Author

I confirm the fix worked, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work regression A bug, but worse
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants