Skip to content

Commit

Permalink
🏁 fix that it might throw a TypeError about reassign a readonly prope…
Browse files Browse the repository at this point in the history
…rty error meesage in Safari (#1408)
  • Loading branch information
kuitos committed Apr 23, 2021
1 parent 4e3e36c commit 8b8f8fa
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 4 deletions.
17 changes: 17 additions & 0 deletions src/sandbox/__tests__/common.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
10 changes: 7 additions & 3 deletions src/sandbox/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export function nextTick(cb: () => void): void {

const fnRegexCheckCacheMap = new WeakMap<any | FunctionConstructor, boolean>();
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;

Expand Down

1 comment on commit 8b8f8fa

@github-actions
Copy link

Choose a reason for hiding this comment

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

Deploy preview for qiankun ready!

✅ Preview
https://qiankun-5129a1fg7-umijs.vercel.app

Built with commit 8b8f8fa.
This pull request is being automatically deployed with vercel-action

Please sign in to comment.