diff --git a/src/sandbox/__tests__/proxySandbox.test.ts b/src/sandbox/__tests__/proxySandbox.test.ts index f2e73b8..223dcda 100644 --- a/src/sandbox/__tests__/proxySandbox.test.ts +++ b/src/sandbox/__tests__/proxySandbox.test.ts @@ -409,34 +409,50 @@ it('native window function calling should always be bound with window', () => { expect(proxy.nativeWindowFunction()).toBe('success'); }); -it('should restore window properties (primitive values) that in whitelisted variables', () => { - const original = { - iframeReady: () => {}, - }; - window.__REACT_ERROR_OVERLAY_GLOBAL_HOOK__ = original; +describe('variables in whitelist', () => { + it('should restore window properties (primitive values) that in whitelisted variables', () => { + const original = { + iframeReady: function t1() {}, + }; + window.__REACT_ERROR_OVERLAY_GLOBAL_HOOK__ = original; - const sandbox = new ProxySandbox('whitelist-variables'); - const { proxy } = sandbox; - sandbox.active(); - proxy.__REACT_ERROR_OVERLAY_GLOBAL_HOOK__ = undefined; - sandbox.inactive(); - proxy.expect(window.__REACT_ERROR_OVERLAY_GLOBAL_HOOK__).toBe(original); -}); - -it('should restore window properties (object descriptors) that in whitelisted variables', () => { - const original = { - iframeReady: () => {}, - }; - Object.defineProperty(window, '__REACT_ERROR_OVERLAY_GLOBAL_HOOK__', { - value: original, + const sandbox = new ProxySandbox('whitelist-variables'); + const { proxy } = sandbox; + sandbox.active(); + proxy.__REACT_ERROR_OVERLAY_GLOBAL_HOOK__ = undefined; + expect(window.__REACT_ERROR_OVERLAY_GLOBAL_HOOK__).toBe(undefined); + sandbox.inactive(); + expect(window.__REACT_ERROR_OVERLAY_GLOBAL_HOOK__).toBe(original); }); - const sandbox = new ProxySandbox('whitelist-variables'); - const { proxy } = sandbox; - sandbox.active(); - proxy.__REACT_ERROR_OVERLAY_GLOBAL_HOOK__ = {}; - sandbox.inactive(); - proxy.expect(window.__REACT_ERROR_OVERLAY_GLOBAL_HOOK__).toBe(original); + it('should restore window properties (object descriptors) that in whitelisted variables', () => { + const original = { + iframeReady: function t2() {}, + }; + Object.defineProperty(window, '__REACT_ERROR_OVERLAY_GLOBAL_HOOK__', { + value: original, + writable: true, + }); + + const sandbox = new ProxySandbox('whitelist-variables'); + const { proxy } = sandbox; + sandbox.active(); + proxy.__REACT_ERROR_OVERLAY_GLOBAL_HOOK__ = {}; + expect(window.__REACT_ERROR_OVERLAY_GLOBAL_HOOK__).toEqual({}); + sandbox.inactive(); + expect(window.__REACT_ERROR_OVERLAY_GLOBAL_HOOK__).toBe(original); + }); + + it('should delete global context while it is undefined before sandbox start', () => { + delete window.__REACT_ERROR_OVERLAY_GLOBAL_HOOK__; + const sandbox = new ProxySandbox('whitelist-variables'); + const { proxy } = sandbox; + sandbox.active(); + proxy.__REACT_ERROR_OVERLAY_GLOBAL_HOOK__ = {}; + expect(window.__REACT_ERROR_OVERLAY_GLOBAL_HOOK__).toEqual({}); + sandbox.inactive(); + expect('__REACT_ERROR_OVERLAY_GLOBAL_HOOK__' in window).toBeFalsy(); + }); }); describe('should work with nest sandbox', () => { diff --git a/src/sandbox/proxySandbox.ts b/src/sandbox/proxySandbox.ts index e1295b6..a9aad44 100644 --- a/src/sandbox/proxySandbox.ts +++ b/src/sandbox/proxySandbox.ts @@ -26,7 +26,7 @@ function uniq(array: Array) { const rawObjectDefineProperty = Object.defineProperty; const variableWhiteListInDev = - process.env.NODE_ENV === 'development' || window.__QIANKUN_DEVELOPMENT__ + process.env.NODE_ENV === 'test' || process.env.NODE_ENV === 'development' || window.__QIANKUN_DEVELOPMENT__ ? [ // for react hot reload // see https://github.com/facebook/create-react-app/blob/66bf7dfc43350249e2f09d138a20840dae8a0a4a/packages/react-error-overlay/src/index.js#L180 @@ -34,7 +34,7 @@ const variableWhiteListInDev = ] : []; // who could escape the sandbox -const variableWhiteList: PropertyKey[] = [ +const globalVariableWhiteList: string[] = [ // FIXME System.js used a indirect call with eval, which would make it scope escape to global // To make System.js works well, we write it back to global window temporary // see https://github.com/systemjs/systemjs/blob/457f5b7e8af6bd120a279540477552a07d5de086/src/evaluate.js#L106 @@ -132,6 +132,9 @@ export default class ProxySandbox implements SandBox { sandboxRunning = true; latestSetProp: PropertyKey | null = null; + // the descriptor of global variables in whitelist before it been modified + globalWhitelistPrevDescriptor: { [p in typeof globalVariableWhiteList[number]]: PropertyDescriptor | undefined } = {}; + active() { if (!this.sandboxRunning) activeSandboxCount++; this.sandboxRunning = true; @@ -144,11 +147,15 @@ export default class ProxySandbox implements SandBox { ]); } - if (--activeSandboxCount === 0) { - variableWhiteList.forEach((p) => { - const descriptor = this.globalWhiteList[p]; - if (descriptor && descriptor.writable) { + if (process.env.NODE_ENV === 'test' || --activeSandboxCount === 0) { + // reset the global value to the prev value + globalVariableWhiteList.forEach((p) => { + const descriptor = this.globalWhitelistPrevDescriptor[p]; + if (descriptor) { Object.defineProperty(this.globalContext, p, descriptor); + } else { + // @ts-ignore + delete this.globalContext[p]; } }); } @@ -157,16 +164,10 @@ export default class ProxySandbox implements SandBox { } globalContext: typeof window; - /** the whitelisted original global variables */ - globalWhiteList: Record; constructor(name: string, globalContext = window) { this.name = name; this.globalContext = globalContext; - this.globalWhiteList = variableWhiteList.reduce((acc, key) => { - acc[key] = Object.getOwnPropertyDescriptor(globalContext, key); - return acc; - }, {} as ProxySandbox['globalWhiteList']); this.type = SandBoxType.Proxy; const { updatedValueSet } = this; @@ -179,24 +180,18 @@ export default class ProxySandbox implements SandBox { set: (target: FakeWindow, p: PropertyKey, value: any): boolean => { if (this.sandboxRunning) { this.registerRunningApp(name, proxy); - // We must kept its description while the property existed in globalContext before + // We must keep its description while the property existed in globalContext before if (!target.hasOwnProperty(p) && globalContext.hasOwnProperty(p)) { const descriptor = Object.getOwnPropertyDescriptor(globalContext, p); const { writable, configurable, enumerable } = descriptor!; - if (writable) { - Object.defineProperty(target, p, { - configurable, - enumerable, - writable, - value, - }); - } + Object.defineProperty(target, p, { configurable, enumerable, writable, value }); } else { - // @ts-ignore target[p] = value; } - if (variableWhiteList.indexOf(p) !== -1) { + // sync the property to globalContext + if (typeof p === 'string' && globalVariableWhiteList.indexOf(p) !== -1) { + this.globalWhitelistPrevDescriptor[p] = Object.getOwnPropertyDescriptor(globalContext, p); // @ts-ignore globalContext[p] = value; } diff --git a/src/utils.ts b/src/utils.ts index d47e789..5362f5f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -88,12 +88,6 @@ export function isCallable(fn: any) { return callable; } -/** - * isPropertyReadonly - * @param target - * @param p - * @returns boolean - */ const frozenPropertyCacheMap = new WeakMap>(); export function isPropertyFrozen(target: any, p?: PropertyKey): boolean { if (!target || !p) {