ProxyHandler no longer implementable in TS 2.4

[amirburbea]

TypeScript Version: 2.4.1

Code

I have a 3rd party component that requires occasionally to think it is receiving an object like an array but with keys as the string indexes of the window of the grid it represents and the values being the row objects. (IE: if I want to pass the grid rows 5-9 of my data source I need to send it

{ "5":{"id":5}, "6":{"id":6}, "7":{"id":7}, "8":{"id":8}, "9":{"id":9} }

I had a proxy handler that allowed me to pass in an array and a start index (so a 5 element array, and a start index of 5 in this example) and the proxy would be seen as this object by the grid.

But the code no longer compiles.

export class DataProxyHandler implements ProxyHandler<{}> {
  private static readonly descriptor: PropertyDescriptor = { configurable: true, enumerable: true, writable: false };

  constructor(public readonly startIndex: number, public readonly data: any[]) {
  }


  enumerate(target: {}) {
    return this.ownKeys(target);
  }

  get(target: {}, property: PropertyKey) {
    if (typeof property !== 'string') {
      return undefined;
    }
    const { startIndex, data } = this, index = parseInt(property);
    return isNaN(index) ? undefined : data[index - startIndex];
  }

  getOwnPropertyDescriptor(target: {}, p: PropertyKey) {
    return typeof p !== 'string' || !this.has(target, p) ? undefined : DataProxyHandler.descriptor;
  }

  getPrototypeOf(target: {}) {
    return Reflect.getPrototypeOf(target);
  }

  has(target: {}, property: PropertyKey) {
    if (typeof property !== 'string') {
      return false;
    }
    const { startIndex, data } = this, index = parseInt(property);
    return !isNaN(index) && index >= startIndex && index < startIndex + data.length;
  }

  isExtensible(target: {}) {
    return false;
  }

  ownKeys(target: {}) {
    const { startIndex, data } = this;
    return data.map((item, index) => (startIndex + index).toString());
  }

  set(target: {}, property: PropertyKey, value: any): boolean {
    throw new Error('Can not set');
  }
}

Expected behavior:

Code would compile

Actual behavior:

Types of property 'getOwnPropertyDescriptor' are incompatible.
Type '(target: {}, p: PropertyKey) => PropertyDescriptor | undefined' is not assignable to type '((target: {}, p: PropertyKey) => PropertyDescriptor) | undefined'.
Type '(target: {}, p: PropertyKey) => PropertyDescriptor | undefined' is not assignable to type '(target: {}, p: PropertyKey) => PropertyDescriptor'.
Type 'PropertyDescriptor | undefined' is not assignable to type 'PropertyDescriptor'.
Type 'undefined' is not assignable to type 'PropertyDescriptor'.

I can get it to compile by specifying the return type of the method getOwnPropertyDescriptor as any.

getOwnPropertyDescriptor(target: {}, p: PropertyKey): any {
    return typeof p !== 'string' || !this.has(target, p) ? undefined : DataProxyHandler.descriptor;
  }

[amirburbea]

I doubt it matters but here is my tsconfig:

{
  "compileOnSave": false,
  "compilerOptions": {
    "allowJs": false,
    "alwaysStrict": true,
    "checkJs": false,
    "declaration": false,
    "emitDecoratorMetadata": false,
    "experimentalDecorators": true,
    "forceConsistentCasingInFileNames": true,
    "importHelpers": true,
    "jsx": "react",
    "module": "commonjs",
    "moduleResolution": "node",
    "noFallthroughCasesInSwitch": true,
    "noImplicitAny": true,
    "noImplicitReturns": true,
    "noImplicitThis": true,
    "noUnusedLocals": true,
    "outDir": "./build",
    "rootDir": "./src",
    "sourceMap": true,
    "strictNullChecks": true,
    "suppressExcessPropertyErrors": false,
    "suppressImplicitAnyIndexErrors": false,
    "target": "es2015",
    "typeRoots": [
      "./src/declarations",
      "./node_modules/@types"
    ]
  },
  "exclude": [
    "./node_modules",
    "./build"
  ]
}

[kitsonk]

This is because of weak type detection. Before the generic of {} in ProxyHandler({}) this is causing the problem. Before TypeScript allowed you to not be specific and {} basically equated to any which is clearly not accurate. You need a type that represents what you are trying to actually proxy. If you can't be specific at design time, you will need to use something like { [key: string]: any}.

[kitsonk]

Actually, based on your example, you could fill in the generic parameter with { [key: string]: { id: number } } I suspect.

[amirburbea]

No, I'm afraid you're wrong. This seems to be because getOwnPropertyDescriptor is an optional method in the interface and in the interface the method is specified as returning PropertyDescriptor | undefined. I definitely do return only those two types but somehow the optional method is screwing up the return type in the interface.

TypeScript 2.3.4 also saw {} as an empty type so it's not that.

The code below still doesn't work

type Target = { [key: string]: any };

export class DataProxyHandler implements ProxyHandler<Target> {
  private static readonly descriptor: PropertyDescriptor = { configurable: true, enumerable: true, writable: false };

  constructor(public readonly startIndex: number, public readonly data: any[]) {
  }

  enumerate(target: Target) {
    return this.ownKeys(target);
  }

  get(target: Target, property: PropertyKey) {
    if (typeof property !== 'string') {
      return undefined;
    }
    const { startIndex, data } = this, index = parseInt(property);
    return isNaN(index) ? undefined : data[index - startIndex];
  }

  getOwnPropertyDescriptor(target: Target, p: PropertyKey) {
    return typeof p !== 'string' || !this.has(target, p) ? undefined : DataProxyHandler.descriptor;
  }

  getPrototypeOf(target: Target) {
    return Reflect.getPrototypeOf(target);
  }

  has(target: Target, property: PropertyKey) {
    if (typeof property !== 'string') {
      return false;
    }
    const { startIndex, data } = this, index = parseInt(property);
    return !isNaN(index) && index >= startIndex && index < startIndex + data.length;
  }

  isExtensible(target: Target) {
    return false;
  }

  ownKeys(target: Target) {
    const { startIndex, data } = this;
    return data.map((item, index) => (startIndex + index).toString());
  }

  set(target: Target, property: PropertyKey, value: any): boolean {
    throw new Error('Can not set');
  }
}

[kitsonk]

TypeScript 2.3.4 also saw {} as an empty type so it's not that.

It saw it as any, 2.4 does not. See: #16047. Also there is strong generic signature checks which are likely making things more challenging to type properly, see #16368. But both of these are regressions in as much as improvements to the type system that let you get away with generic abuse.

[amirburbea]

To be honest, I believe (based on the fact that the updated example still doesn't compile), the whole of the problem is simply lib.es6.d.ts having getOwnPropertyDescriptor? (target: T, p: PropertyKey): PropertyDescriptor; instead of getOwnPropertyDescriptor? (target: T, p: PropertyKey): PropertyDescriptor | undefined; because that is more consistent with the ProxyHandler spec from es2015.

Let's just PR getOwnPropertyDescriptor to allow undefined as a return type of the method and call it a day.

This has nothing to do with the T in ProxyHandler<T>

[amirburbea]

Since I've never done a PR for this project, can I assume that you will? Like I said it's just lib.es6.d.ts that needs a change as well as lib.es2015.proxy.d.ts

[kitsonk]

Since I've never done a PR for this project, can I assume that you will?

I am not part of the TypeScript team.

[amirburbea]

OK - this gets more confusing actually.

Well I went to do it myself, and weirdly enough it looks like it was done and merged into master on May 22 - https://github.com/Microsoft/TypeScript/blob/abd73c2e013a859779fd6c9a047e996ea704c0e6/lib/lib.es2015.proxy.d.ts

and the lib.*.d.ts fixes are there in typescript 2.4.1

So, lib.es6.d.ts was fixed in TypeScript 2.4.1 but this still won't build. Why?

Well, I feel like a total jack*ss but it actually was that I had a global install of typescript that was older so when I ran tsc it was running from 2.3.4 (with the old lib) and not 2.4.1 (with the fixed lib)

Sorry to waste everyone's time

[RyanCavanaugh]

@amirburbea no worries, thanks for following up