Re-exporting the Record module creates a runtime exception

[Magellol]

🐛 Bug report

Current Behavior

We usually "extend" some modules with our own operators and we do this by re-exporting the module from another file such as:

// modules/Record/index.ts
export * from 'fp-ts/lib/Record';
export * from './operators';

// modules/Record/operators.ts
import * as R from 'fp-ts/lib/Record';

export const fn = () => {}

So in the codebase, we can do:

import * as R from 'modules/Record';

R.fn();
R.keys();
...

We've been doing that for different modules such as Option, Array, Either etc and all is good but for some reason re-exporting Record creates the following runtime exception

Expected behavior

No runtime exception. Allowing us to re-export the module to extend it.

Your environment

Software	Version(s)
fp-ts	2.6.5
TypeScript	3.9.2

[Magellol]

@OliverJAsh ran some tests and it may potentially be because of the following code generated by TS

var __exportStar = (this && this.__exportStar) || function(m, exports) {
    for (var p in m) if (p !== "default" && !exports.hasOwnProperty(p)) __createBinding(exports, m, p);
}

https://www.typescriptlang.org/play/index.html?module=1&ssl=1&ssc=1&pln=1&pc=20#code/KYDwDg9gTgLgBAKjgMyhAtnA5MiEtA

In this case, it feels like the export.hasOwnProperty used by the snippet is pointing at the hasOwnProperty from ReadOnlyRecord

fp-ts/src/ReadonlyRecord.ts

Lines 162 to 164 in 1b0ee81

	export function hasOwnProperty<K extends string>(k: string, r: ReadonlyRecord<K, unknown>): k is K {
	return _hasOwnProperty.call(r, k)
	}

Looks like renaming it should fix the issue

Ref on the TS repo: microsoft/TypeScript#3197

[gcanti]

@OliverJAsh RE: the "bug" label. It doesn't look fp-ts's fault

[OliverJAsh]

Yeah you're right. Although the TS team have already said they won't fix this, so I guess it's on us to workaround it instead? microsoft/TypeScript#3197 (comment)

[gcanti]

the TS team have already said they won't fix this

I see. I can't rename hasOwnProperty because would be a breaking change.

However, as a fix, I can try to handle this

const _hasOwnProperty = Object.prototype.hasOwnProperty

export function hasOwnProperty<K extends string>(k: string, r: ReadonlyRecord<K, unknown>): k is K
export function hasOwnProperty<K extends string>(this: any, k: string, r?: ReadonlyRecord<K, unknown>): k is K {
  return _hasOwnProperty.call(r === undefined ? this : r, k)
}

what do you think?

[OliverJAsh]

That should work!

[OliverJAsh]

This bug is happening again in 2.10.0 when we try to call Record.filterMap. It looks like it's because of this change: 02e0d21#diff-2a28f9bcbc06ac11c7768967193244d8f9ca05a4e96225ca8449930aa26870b0R663.

We need to apply the same fix to hasOwnProperty here:

fp-ts/src/internal.ts

Line 28 in 6a38637

export const hasOwnProperty = Object.prototype.hasOwnProperty

I'll raise a PR.

[gcanti]

@OliverJAsh damn, I forgot again this issue, I'm sorry :(

This is an internal function so we can rename it (to has for example) without any worries

[OliverJAsh]

Unfortunately the issue persists in 2.10.1. I think this change is also causing an issue: 281dc29#diff-dc4b4fb0cd6e06c8bf587eb2f8817ffa1ed096efe2c6c5175d3604fb57d3a96cR796

Record.hasOwnProperty no longer handles this because it's now just an alias for RR.has, and RR.has doesn't handle this anymore: 281dc29#diff-2a28f9bcbc06ac11c7768967193244d8f9ca05a4e96225ca8449930aa26870b0L168

I'm not sure how we should fix this. WDYT @gcanti?