Skip to content

Creating a strongly typed Array wrapper duplicates code #12013

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

Closed
alfaproject opened this issue Nov 2, 2016 · 7 comments
Closed

Creating a strongly typed Array wrapper duplicates code #12013

alfaproject opened this issue Nov 2, 2016 · 7 comments
Labels
Fixed A PR has been merged for this issue

Comments

@alfaproject
Copy link

alfaproject commented Nov 2, 2016

TypeScript Version:
Playground

Code

interface IMyArray<T> extends Array<T> {
    first: T;
    reset: () => void;
}

class MyArray<T> extends Array<T> implements IMyArray<T> {
    constructor() {
        super();

        const _arr = [] as MyArray<T>;

        // Property: first
        Object.defineProperty(_arr, 'first', {
            get: () => _arr[0],
            enumerable: true,
            configurable: true
        })

        // Method: reset
        _arr.reset = () => _arr.length = 0;

        return _arr;
    }

    get first(): T { return; }
    reset() { return; }
}

const a = new MyArray<number>();
a.push(...[1, 2, 3]);
alert(a instanceof Array); // true
alert(a.length); // 3
alert(a.first); // 1
a.length = 0;
alert(a.length); // 0
alert(a[0]); // undefined

Expected behavior:
Allow to proper wrap and extend an Array without the custom interface.

Actual behavior:
To create a strongly typed subclassed array, this is the only solution I found, and I think TypeScript can do much better syntax sugar than this.

The default subclassing of an Array doesn't actually create a proper Array, so I don't see the point in that, but my version does despite all of the extra code, so I don't see why it can't be used instead.

@kitsonk
Copy link
Contributor

kitsonk commented Nov 3, 2016

You are heavily crossing the functional/syntactic boundary here. In ES5, built-ins are not subclassable. This was only introduced in ES6.

Are you suggesting that TypeScript down emit special handling for builtins when trying to subclass? I suspect this had been avoided, because like in all things, TypeScript only focuses on syntactical transforms, not functional polyfills. This certainly seems like the domain of something built upon TypeScript or JavaScript, just like you have done.

@alfaproject
Copy link
Author

alfaproject commented Nov 3, 2016

The problem is that I can't even do that using proper ES5 code, because I can't have it typed, so I need to plaster any casts everywhere. ):

Example of what I want to do in ES5:

function MyArray() {
  var _arr = [];

  // Property: first
  Object.defineProperty(_arr, 'first', {
      get: () => _arr[0],
      enumerable: true,
      configurable: true
  });

  // Method: reset
  _arr.reset = function() { _arr.length = 0; };

  return _arr;
}

const a = new MyArray<number>();
a.push(...[1, 2, 3]);
alert(a instanceof Array); // true
alert(a.length); // 3
alert(a.first); // 1
a.length = 0;
alert(a.length); // 0
alert(a[0]); // undefined

All I really want is for that to be strongly typed, and my example on the original post is the only thing I could come up with. ):

P.S.: This won't even work in TS, because only a void function can be called with the new keyword. I don't get the lack of backward compatibility in this case.

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Nov 4, 2016

Subclassing builtins is a complete mess, and in my mind largely a failure on the part of TC39 for having invested so much time and energy into introducing a class model as classical sugar over prototypes and yet failing to make the upgrade semantics reasonable or consistent, or the feature powerful enough to be worth the trouble.

If I write this code today

class MyMap<K, V> extends Map<K, V> {
    constructor(entries: ([K, V])[]) {
      super(entries);
    }
    map<R>(f: ([key, value]: [K, V]) => R) {
      return this.entries.map(f);    
    }
}

transpile it with --target es5, and ship it with well a behaved pollyfill (one that doesn't execute if its augmentations are already present in the runtime), I will get a TypeError in Chrome and numerous other environments because the emitted code will try to call super. Conversely in older runtimes, it will not explode and will behave intuitively.

@alfaproject
Copy link
Author

The thing is, the compiler knows the target and is already poly filling the subclassing anyway, so we could actually have 2 ways to 'subclass' an Array and keep the magic properties:

  1. target=es5-, wrap a factory method
  2. target=es6+, just use native class

I think that for ES5, people would expect to keep the magic properties of the Array if you are 'subclassing' it, so I don't think it would be a big deal. async/await is a much bigger poly fill than this would be, even if it would be under a flag.

@kitsonk
Copy link
Contributor

kitsonk commented Nov 7, 2016

async/await is a much bigger poly fill than this would be, even if it would be under a flag.

The big difference is that async/await is syntactical and therefore something that TypeScript feels they need to address. The polyfill doesn't provide a Promise implementation either, that is something you have to provide. Polyfilling a subsclassable Array is far more just a functional polyfill and something the TypeScript team is unlikely to fit the design goals of TypeScript.

@alfaproject
Copy link
Author

So, I had another go at this, and finally came up with a better hack:

class MyArray<T> extends Array<T> {
    constructor() {
        const _arr: MyArray<T> = <any>super();

        // Property: first
        Object.defineProperty(_arr, 'first', {
            get: () => _arr[0],
            enumerable: true,
            configurable: true
        });

        // Method: reset
        _arr.reset = () => _arr.length = 0;

        return _arr;
    }

    readonly first: string;
    reset: () => void;
}

const a = new MyArray<number>();
a.push(...[1, 2, 3]);
alert(a instanceof Array); // true
alert(a.length); // 3
alert(a.first); // 1
a.length = 0;
alert(a.length); // 0
alert(a[0]); // undefined

This doesn't duplicate generated code.
It's not ideal, but it's one step closer to my needs.

@mhegazy
Copy link
Contributor

mhegazy commented Dec 14, 2016

TS 2.1 changes allow for extending built-ins without the duplication. the prototype needs to be set manually though. see https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#extending-built-ins-like-error-array-and-map-may-no-longer-work for more details.

@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Dec 14, 2016
@mhegazy mhegazy closed this as completed Dec 14, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

4 participants