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

fix(helpers): Use hasOwnProperty.call #39537

Merged
merged 3 commits into from
Jul 10, 2020

Conversation

rbuckton
Copy link
Member

@rbuckton rbuckton commented Jul 9, 2020

This replaces #37013 as I was unable to push up changes to that PR. Thanks @ExE-Boss!

From @ExE-Boss:

Currently, the following code:

exports.hasOwnProperty(…) override

// func-util.ts
export const { apply, call } = (({ apply }) => ({
	apply: <T, A extends any[], R>(
		target: (this: T, ...args: A) => R,
		thisArg: T,
		args: A = [] as A
	): R => apply(target, thisArg, args),
	call: <T, A extends any[], R>(
		target: (this: T, ...args: A) => R,
		thisArg: T,
		...args: A
	): R => apply(target, thisArg, args),
}))(Reflect);
// helpers.ts
import { assertArgs, assertObject } from "./types.js";
import { call } from "./func-util.js";

export * from "./func-util.js";
export const {
	hasOwnProperty,
	isPrototypeOf,
} = (({ hasOwnProperty, isPrototypeOf }) => {
	return {
		hasOwnProperty(target: object, property: PropertyKey) {
			assertArgs(arguments.length, 2);
			assertObject(target);
			return call(hasOwnProperty, target, property);
		},
		isPrototypeOf(proto: object, target: any) {
			assertArgs(arguments.length, 2);
			assertObject(proto);
			return call(isPrototypeOf, proto, target);
		},
	};
})(Object.prototype);

results in an error at runtime when transpiled to non‑ESM, because exports.hasOwnProperty(…) is called with only a single non‑object argument.


This also won't work in case exports has its prototype set to null


TSLib PR: microsoft/tslib#92

Replaces #37013
Fixes #37016
Fixes #3197

ExE-Boss and others added 3 commits February 25, 2020 14:13
# Conflicts:
#	tests/baselines/reference/tsbuild/sample1/incremental-declaration-changes/when-esModuleInterop-option-changes.js
#	tests/baselines/reference/tsc/declarationEmit/initial-build/when-pkg-references-sibling-package-through-indirect-symlink.js
Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

Yeah, I was always confused why some helpers did use the prototype method and some did not.

@rbuckton rbuckton merged commit 87fd182 into master Jul 10, 2020
@rbuckton
Copy link
Member Author

The tslib side of this change has been merged as well.

@rbuckton rbuckton deleted the fix/helpers/use-hasownproperty-call branch July 11, 2020 00:13
rimunroe added a commit to rimunroe/vitest-commonjs-exports-prototype-issue that referenced this pull request Jan 26, 2023
rimunroe added a commit to rimunroe/vitest-commonjs-exports-prototype-issue that referenced this pull request Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid calling hasOwnProperty in helpers Re-export issue with hasOwnProperty.
4 participants