Skip to content
This repository has been archived by the owner on Jul 6, 2018. It is now read-only.

Typings for ember prototype extensions #8

Closed
dwickern opened this issue Aug 13, 2017 · 10 comments
Closed

Typings for ember prototype extensions #8

dwickern opened this issue Aug 13, 2017 · 10 comments

Comments

@dwickern
Copy link
Collaborator

Ember modifies the prototype for String, Function and Array based on EmberENV.EXTEND_PROTOTYPES

// function prototype is used for computed properties
function() {
  return this.get('name')
}.property('name')

// string prototype is used for random stuff
'test'.capitalize()

// array prototype is used for various functionality
['test'].get('firstObject')
arr.findBy('id', 5)

I'd like to start a discussion about which typings we should provide for these

@dwickern
Copy link
Collaborator Author

I think we at least need the Array prototype extensions. The guides don't wrap arrays in Ember.A() and I don't think people do that in practice either. I had typings for arrays in #1 but temporarily removed them since there were signature conflicts with the built-in methods (forEach, find, map, reduce, etc.).

@dfreeman
Copy link
Member

Also perhaps noteworthy, Ember functions that declare parameters of type Function are currently unusable because they're capturing the prototype extensions without declaring that the global Function implements them:

image

@dfreeman
Copy link
Member

My take is that we probably don't want to include typings for the String and Function prototype extensions out of the box. There's an RFC regarding the future of Ember.String that still has some open questions, but it seems almost certain the prototype extensions will be deprecated. The guides also migrated away from using the function prototype extensions as well a while back, though I don't know of an explicit call to deprecate them at this time.

As you pointed out, the array extensions seem to be the ones still widely in use. I had a branch with typings for arrays that agreed with the built-in Array<T> method signatures that was based on #11. I'll aim to rebase and put that up for review sometime in the next couple days as a starting point.

@dwickern
Copy link
Collaborator Author

I think even if they're not exported by default, we should have the prototype extensions available if someone wants to opt-in. For example you could put something like this into your app.d.ts:

declare global {
  interface Function extends Ember.FunctionPrototypeExtensions {}
}

Maybe it should be the same for Array.prototype, so that addons have type-checking to make sure they use Ember.A()

@chriskrycho
Copy link
Member

Maybe we could have something like an ember-prototype-extensions bit that merges the interfaces, but isn't shipped by default?

@dfreeman
Copy link
Member

Since different applications might want to opt into different combinations of prototype extensions, one thing I had been playing with locally was having types/ember/{array,function,string}-prototype-extensions.d.ts files, and then switching from exclude to include in my playground app's tsconfig.json:

"include": [
  "app/**/*.ts",
  "tests/**/*.ts",
  "node_modules/ember-typings/types/ember/array-prototype-extensions.d.ts"
]

I don't think DT is overly fond of having little extra files floating around for a single lib, though?

@dwickern
Copy link
Collaborator Author

If we went with #8 (comment), the ember-cli-typescript blueprint could generate some boilerplate like this

// uncomment to enable prototype extensions
/*
declare global {
  interface Function extends Ember.FunctionPrototypeExtensions {}
  interface Array<T> extends Ember.NativeArray<T> {}
  interface String extends Ember.String {}
}
*/

@dfreeman
Copy link
Member

dfreeman commented Aug 29, 2017

👍 Much simpler. I like that. (Sorry, I'm not sure I actually properly read #8 (comment) the first time through)

@chriskrycho
Copy link
Member

I'm in favor of that approach as well, and in general I actually want as few things as possible ending up on DT. That's a last resort.

@dwickern
Copy link
Collaborator Author

I added function prototype extensions in #33

Array prototype extensions seem to be working since #26 and I added some tests in #35

I'm not going to implement string prototype extensions, but I'll accept a PR for it

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants