From 60ce3982cc581c83f33f60c2671890319564b9a1 Mon Sep 17 00:00:00 2001 From: Kuitos Date: Fri, 23 Apr 2021 17:13:54 +0800 Subject: [PATCH] =?UTF-8?q?=F0=9F=8F=81=20fix=20that=20it=20might=20throw?= =?UTF-8?q?=20a=20TypeError=20about=20reassign=20a=20readonly=20property?= =?UTF-8?q?=20error=20meesage=20in=20Safari=20(#1408)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/sandbox/__tests__/common.test.ts | 17 +++++++++++++++++ src/sandbox/common.ts | 10 +++++++--- src/utils.ts | 2 +- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/sandbox/__tests__/common.test.ts b/src/sandbox/__tests__/common.test.ts index 788e6a8..4030a9d 100644 --- a/src/sandbox/__tests__/common.test.ts +++ b/src/sandbox/__tests__/common.test.ts @@ -40,4 +40,21 @@ describe('getTargetValue', () => { // window.field not be affected expect(window.field).toEqual('123'); }); + + it('should work well while value have a readonly prototype on its prototype chain', () => { + function callableFunction() {} + + const functionWithReadonlyPrototype = () => {}; + Object.defineProperty(functionWithReadonlyPrototype, 'prototype', { + writable: false, + enumerable: false, + configurable: false, + value: 123, + }); + + Object.setPrototypeOf(callableFunction, functionWithReadonlyPrototype); + + const boundFn = getTargetValue(window, callableFunction); + expect(boundFn.prototype).toBe(callableFunction.prototype); + }); }); diff --git a/src/sandbox/common.ts b/src/sandbox/common.ts index 0a4155f..4e8633b 100644 --- a/src/sandbox/common.ts +++ b/src/sandbox/common.ts @@ -29,10 +29,14 @@ export function getTargetValue(target: any, value: any): any { boundValue[key] = value[key]; } - // 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 + // copy prototype if bound function not have but target one have + // as prototype is non-enumerable mostly, we need to copy it from target function manually if (value.hasOwnProperty('prototype') && !boundValue.hasOwnProperty('prototype')) { - boundValue.prototype = value.prototype; + // we should not use assignment operator to set boundValue prototype like `boundValue.prototype = value.prototype` + // as the assignment will also look up prototype chain while it hasn't own prototype property, + // when the lookup succeed, the assignment will throw an TypeError like `Cannot assign to read only property 'prototype' of function` if its descriptor configured with writable false or just have a getter accessor + // see https://github.com/umijs/qiankun/issues/1121 + Object.defineProperty(boundValue, 'prototype', { value: value.prototype, enumerable: false, writable: true }); } return boundValue; diff --git a/src/utils.ts b/src/utils.ts index 7e6ebd1..f60a0ed 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -24,7 +24,7 @@ export function nextTick(cb: () => void): void { const fnRegexCheckCacheMap = new WeakMap(); export function isConstructable(fn: () => any | FunctionConstructor) { - // prototype methods might be added while code running, so we need check it every time + // prototype methods might be changed while code running, so we need check it every time const hasPrototypeMethods = fn.prototype && fn.prototype.constructor === fn && Object.getOwnPropertyNames(fn.prototype).length > 1;