From 1f97acfc81a19a5c40905a95857b60fd01c3c74c Mon Sep 17 00:00:00 2001 From: Kuitos Date: Tue, 13 Apr 2021 14:24:48 +0800 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20constructable=20checking=20shoul?= =?UTF-8?q?d=20not=20be=20cached=20if=20the=20function=20prototype=20funct?= =?UTF-8?q?ion=20was=20added=20after=20first=20running=20(#1381)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/prefetch.ts | 1 - src/sandbox/__tests__/common.test.ts | 43 ++++++++++++++++++++++++ src/sandbox/common.ts | 10 ++---- src/utils.ts | 50 ++++++++++++++++++++-------- 4 files changed, 81 insertions(+), 23 deletions(-) create mode 100644 src/sandbox/__tests__/common.test.ts diff --git a/src/prefetch.ts b/src/prefetch.ts index 5e3297b45..0c4e28350 100644 --- a/src/prefetch.ts +++ b/src/prefetch.ts @@ -28,7 +28,6 @@ declare global { cancelIdleCallback: (handle: RequestIdleCallbackHandle) => void; } - // eslint-disable-next-line @typescript-eslint/consistent-type-definitions interface Navigator { connection: { saveData: boolean; diff --git a/src/sandbox/__tests__/common.test.ts b/src/sandbox/__tests__/common.test.ts new file mode 100644 index 000000000..788e6a8c0 --- /dev/null +++ b/src/sandbox/__tests__/common.test.ts @@ -0,0 +1,43 @@ +/** + * @author Kuitos + * @since 2021-04-12 + */ + +import { getTargetValue } from '../common'; + +describe('getTargetValue', () => { + it('should work well', () => { + const a1 = getTargetValue(window, undefined); + expect(a1).toEqual(undefined); + + const a2 = getTargetValue(window, null); + expect(a2).toEqual(null); + + const a3 = getTargetValue(window, function bindThis(this: any) { + return this; + }); + const a3returns = a3(); + expect(a3returns).toEqual(window); + }); + + it('should work well while function added prototype methods after first running', () => { + function prototypeAddedAfterFirstInvocation(this: any, field: string) { + this.field = field; + } + const notConstructableFunction = getTargetValue(window, prototypeAddedAfterFirstInvocation); + // `this` of not constructable function will be bound automatically, and it can not be changed by calling with special `this` + const result = {}; + notConstructableFunction.call(result, '123'); + expect(result).toStrictEqual({}); + expect(window.field).toEqual('123'); + + prototypeAddedAfterFirstInvocation.prototype.addedFn = () => {}; + const constructableFunction = getTargetValue(window, prototypeAddedAfterFirstInvocation); + // `this` coule be available if it be predicated as a constructable function + const result2 = {}; + constructableFunction.call(result2, '456'); + expect(result2).toStrictEqual({ field: '456' }); + // window.field not be affected + expect(window.field).toEqual('123'); + }); +}); diff --git a/src/sandbox/common.ts b/src/sandbox/common.ts index 0a97afaf2..0a4155f23 100644 --- a/src/sandbox/common.ts +++ b/src/sandbox/common.ts @@ -14,13 +14,7 @@ export function setCurrentRunningSandboxProxy(proxy: WindowProxy | null) { currentRunningSandboxProxy = proxy; } -const functionBoundedValueMap = new WeakMap(); export function getTargetValue(target: any, value: any): any { - const cachedBoundFunction = functionBoundedValueMap.get(value); - if (cachedBoundFunction) { - return cachedBoundFunction; - } - /* 仅绑定 isCallable && !isBoundedFunction && !isConstructable 的函数对象，如 window.console、window.atob 这类。目前没有完美的检测方式，这里通过 prototype 中是否还有可枚举的拓展方法的方式来判断 @warning 这里不要随意替换成别的判断方式，因为可能触发一些 edge case（比如在 lodash.isFunction 在 iframe 上下文中可能由于调用了 top window 对象触发的安全异常） @@ -37,10 +31,10 @@ export function getTargetValue(target: any, value: any): any { // copy prototype if bound function not have // mostly a bound function have no own prototype, but it not absolute in some old version browser, see https://github.com/umijs/qiankun/issues/1121 - if (value.hasOwnProperty('prototype') && !boundValue.hasOwnProperty('prototype')) + if (value.hasOwnProperty('prototype') && !boundValue.hasOwnProperty('prototype')) { boundValue.prototype = value.prototype; + } - functionBoundedValueMap.set(value, boundValue); return boundValue; } diff --git a/src/utils.ts b/src/utils.ts index b9abdac06..7e6ebd136 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -22,21 +22,34 @@ export function nextTick(cb: () => void): void { Promise.resolve().then(cb); } -const constructableMap = new WeakMap(); +const fnRegexCheckCacheMap = new WeakMap(); export function isConstructable(fn: () => any | FunctionConstructor) { - if (constructableMap.has(fn)) { - return constructableMap.get(fn); + // prototype methods might be added while code running, so we need check it every time + const hasPrototypeMethods = + fn.prototype && fn.prototype.constructor === fn && Object.getOwnPropertyNames(fn.prototype).length > 1; + + if (hasPrototypeMethods) return true; + + if (fnRegexCheckCacheMap.has(fn)) { + return fnRegexCheckCacheMap.get(fn); } - const constructableFunctionRegex = /^function\b\s[A-Z].*/; - const classRegex = /^class\b/; + /* + 1. 有 prototype 并且 prototype 上有定义一系列非 constructor 属性 + 2. 函数名大写开头 + 3. class 函数 + 满足其一则可认定为构造函数 + */ + let constructable = hasPrototypeMethods; + if (!constructable) { + // fn.toString has a significant performance overhead, if hasPrototypeMethods check not passed, we will check the function string with regex + const fnString = fn.toString(); + const constructableFunctionRegex = /^function\b\s[A-Z].*/; + const classRegex = /^class\b/; + constructable = constructableFunctionRegex.test(fnString) || classRegex.test(fnString); + } - // 有 prototype 并且 prototype 上有定义一系列非 constructor 属性，则可以认为是一个构造函数 - const constructable = - (fn.prototype && fn.prototype.constructor === fn && Object.getOwnPropertyNames(fn.prototype).length > 1) || - constructableFunctionRegex.test(fn.toString()) || - classRegex.test(fn.toString()); - constructableMap.set(fn, constructable); + fnRegexCheckCacheMap.set(fn, constructable); return constructable; } @@ -47,9 +60,18 @@ export function isConstructable(fn: () => any | FunctionConstructor) { * We need to discriminate safari for better performance */ const naughtySafari = typeof document.all === 'function' && typeof document.all === 'undefined'; -export const isCallable = naughtySafari - ? (fn: any) => typeof fn === 'function' && typeof fn !== 'undefined' - : (fn: any) => typeof fn === 'function'; +const callableFnCacheMap = new WeakMap(); +export const isCallable = (fn: any) => { + if (callableFnCacheMap.has(fn)) { + return true; + } + + const callable = naughtySafari ? typeof fn === 'function' && typeof fn !== 'undefined' : typeof fn === 'function'; + if (callable) { + callableFnCacheMap.set(fn, callable); + } + return callable; +}; const boundedMap = new WeakMap(); export function isBoundedFunction(fn: CallableFunction) {