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

🎨 optimize current proxy setting logic #1486

Merged
merged 2 commits into from
Jul 27, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion src/sandbox/__tests__/proxySandbox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,11 @@ test('hasOwnProperty should works well', () => {
test('descriptor of non-configurable and non-enumerable property existed in raw window should be the same after modified in sandbox', () => {
Object.defineProperty(window, 'nonConfigurableProp', { configurable: false, writable: true });
// eslint-disable-next-line getter-return
Object.defineProperty(window, 'nonConfigurablePropWithAccessor', { configurable: false, get() {}, set() {} });
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个换行是啥规则, 参数数量超过某一值换行?

Copy link
Member Author

Choose a reason for hiding this comment

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

看上去是的

Copy link
Member Author

Choose a reason for hiding this comment

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

应该是 prettier 的 maxWidth

Object.defineProperty(window, 'nonConfigurablePropWithAccessor', {
configurable: false,
get() {},
set() {},
});
Object.defineProperty(window, 'enumerableProp', { enumerable: true, writable: true });
Object.defineProperty(window, 'nonEnumerableProp', { enumerable: false, writable: true });

Expand Down Expand Up @@ -297,6 +301,7 @@ test('the prototype should be kept while we create a function with prototype on
const proxy = new ProxySandbox('new-function').proxy as any;

function test() {}

proxy.fn = test;
expect(proxy.fn === test).toBeFalsy();
expect(proxy.fn.prototype).toBe(test.prototype);
Expand Down Expand Up @@ -347,3 +352,15 @@ it('should return true while [[GetPrototypeOf]] invoked by proxy object', () =>
expect(Reflect.getPrototypeOf(proxy)).toBe(Reflect.getPrototypeOf(window));
expect(Reflect.getPrototypeOf(proxy)).toBe(Object.getPrototypeOf(window));
});

it('should get current running sandbox proxy correctly', async () => {
const { proxy } = new ProxySandbox('running');

await Promise.resolve().then(() => {
expect(getCurrentRunningSandboxProxy()).toBeNull();
// @ts-ignore
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const unused = proxy.accessing;
expect(getCurrentRunningSandboxProxy()).toBe(proxy);
});
});
2 changes: 1 addition & 1 deletion src/sandbox/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { isBoundedFunction, isCallable, isConstructable } from '../utils';

let currentRunningSandboxProxy: WindowProxy | null;
let currentRunningSandboxProxy: WindowProxy | null = null;
export function getCurrentRunningSandboxProxy() {
return currentRunningSandboxProxy;
}
Expand Down
11 changes: 6 additions & 5 deletions src/sandbox/proxySandbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,12 @@ export default class ProxySandbox implements SandBox {
},

get(target: FakeWindow, p: PropertyKey): any {
setCurrentRunningSandboxProxy(proxy);
// FIXME if you have any other good ideas
// remove the mark in next tick, thus we can identify whether it in micro app or not
// this approach is just a workaround, it could not cover all complex cases, such as the micro app runs in the same task context with master in some case
nextTick(() => setCurrentRunningSandboxProxy(null));

if (p === Symbol.unscopables) return unscopables;

// avoid who using window.window or window.self to escape the sandbox environment to touch the really window
Expand Down Expand Up @@ -247,11 +253,6 @@ export default class ProxySandbox implements SandBox {

// mark the symbol to document while accessing as document.createElement could know is invoked by which sandbox for dynamic append patcher
if (p === 'document' || p === 'eval') {
setCurrentRunningSandboxProxy(proxy);
// FIXME if you have any other good ideas
// remove the mark in next tick, thus we can identify whether it in micro app or not
// this approach is just a workaround, it could not cover all complex cases, such as the micro app runs in the same task context with master in some case
nextTick(() => setCurrentRunningSandboxProxy(null));
switch (p) {
case 'document':
return document;
Expand Down