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

[DRAFT!!!] QUEST: Let's type the Ember.js ecosystem! #48

Closed
8 of 98 tasks
chriskrycho opened this issue Jul 27, 2017 · 17 comments
Closed
8 of 98 tasks

[DRAFT!!!] QUEST: Let's type the Ember.js ecosystem! #48

chriskrycho opened this issue Jul 27, 2017 · 17 comments
Labels
deferred We have a specific reason for delaying implementation of this (but do plan to!) enhancement help wanted types:core Something is wrong with the Ember type definitions

Comments

@chriskrycho
Copy link
Member

chriskrycho commented Jul 27, 2017

*DRAFT – STILL WORKING OUT THE DETAILS HERE! DO NOT JUMP IN JUST YET!

Let’s type the Ember.js ecosystem!

Overview

The goal of this quest issue is to write TypeScript type definitions to cover the top 100 addons listed on Ember Observer as of the time the quest is launched. Our bonus goal is to type any other addons which want to participate, as well!

Philosophy

There are two overarching concerns that drive everything else in this quest:

  1. Good type definitions are worth their weight in gold. Bad type definitions are worse than no type definitions at all. Accordingly, our goal is not just to get some type definitions written, but to write good type definitions that actually give us these benefits, and which don’t mislead us or give us a false sense of security.

  2. We should be good citizens of the Ember.js community. We want to use TypeScript to be helpful; we do not want to be pushy jerks or typed-language fanatics. As part of the Ember.js ecosystem, we’re all in this together, and those of us working on the TypeScript side should never look down on people who prefer JavaScript – there are legitimate tradeoffs either direction. So, as always, let’s be kind!

Improving developer experience for all Ember.js users

By doing this, we can make the experience of developing Ember apps better for everyone – not just TypeScript users, but all the people using plain-old JavaScript as well. An increasing number of editors will take advantage of TypeScript type definitions if they’re available to help you in your development experience, even if you’re not using TypeScript at all. And of course, if you are using TypeScript, having these will make all the difference in your experience of using an addon. So let’s do it!

How to participate

First, a couple notes to both add-on authors and add-on users, then the nitty gritty!

Addon authors

{>> TODO <<}

If you are open to someone else adding typings for your addon

One other approach you can take, if you're interested in adding types to your addon but don't have the knowledge or bandwidth to do it yourself is to add a Help Wanted issue to your repository and link it here! If your repo isn't in the top-100-addons list, I'll add it anyway in its own section. If you're

{>> TODO <<}

Add-on users

If you take a look at one of the addons on this list and the author hasn’t yet expressed any interest in adding TypeScript or TypeScript definitions to their addon, the very first thing you should do is open an issue and see if they’re open to the possibility of converting their addon to TypeScript or hosting the type definitions in the addon. We strongly recommend you just use this template when opening the issue:

  • Suggested Title: Interested in TypeScript?

  • Suggested Body:

    Hello! I use your addon and really appreciate it. I’m also participating in this quest issue to add TypeScript support throughout the Ember.js ecosystem. This will benefit to everyone who uses your addon, not just TypeScript users! (See here for an explanation.)

    Are you interested in either converting the addon to use TypeScript itself, or in adding type definitions? I’d love to help out, if so! And if not, that’s just fine. Thanks!

    To make it easy, you can just copy and paste this directly into the issue of a body:

    Hello! I use your addon and really appreciate it. I’m also participating in [this quest issue to add TypeScript support throughout the Ember.js ecosystem](https://github.com/typed-ember/ember-typings/issues/14). This will benefit to *everyone* who uses your addon, not just TypeScript users! (See [here]( improving-developer-experience-for-all-emberjs-users) for an explanation.)
    
    Are you interested in either converting the addon to use TypeScript itself, or in adding type definitions? I’d love to help out, if so! And if not, that’s just fine. Thanks!
    

Based on their answer, you can then help them convert their addon to use TypeScript, help them add type definitions to the repository, or – and this is very important – just leave them alone if they’re not interested! It’s perfectly fine if addon authors aren’t interested in using TypeScript themselves or if they don’t want to take on the extra maintenance burden of hosting type definitions. Do not badger an addon author about TypeScript! If they aren’t interested, we can fall back to using DefinitelyTyped to host type definitions!

Actually doing the work

Okay, let's dive in and talk about how to do this!

Converting an addon to TypeScript

{>> TODO <<}

Writing type definitions

As noted above, the most important thing about type definitions is that they are correct. If they're incorrect, they'll actually lead users to write wrong code, and that's a deeply frustrating experience.

Some general tips:

  1. Wherever there is documentation, read it very carefully. It's not always correct or up to date, but it still usually gives you a good starting point. If nothing else, it is often a good guide for what the public API is.

  2. Read the source code for the public API. This is the only way to actually be sure of what the code does. Take a look at the function arguments, and look at what is done with them. Are there optional arguments? Are the arguments "polymorphic," i.e. can you pass in either a string or an array of strings or an object as the nth argument? Does it always return the same thing or not? Etc.

  3. Pick something you can handle! Your type definitions should capture everything about the actual behavior of an addon's classes and functions. Sometimes, this will be easy. Other times (here's looking at you, Ember CLI Mirage!) this will involve a lot of arcane type mechanics. If you're just starting out, that's okay! Pick a simpler addon. If you've got mad TypeScript chops, awesome: pick something harder!

  4. Don't go it alone. Get input along the way. The folks on the ember-cli-typescript team and many of the people hanging out in #topic-typescript on the Ember Community Slack are happy to help review your typings work and help you go from just wrote my first type definition ever! to mentoring other people! And getting feedback along the way will help us achieve our goal

{>> TODO: how to write a good type definition <<}

Type definition basics

{>> TODO: .d.ts files, modules, exports <<}

Writing good type definitions

For more complicated types, you may need to lean on TypeScript's optionals, generics, union and intersection types, overloading, or the combination of several of those.

Optionals

One of the most common scenarios you'll need to cover is handling optional arguments or properties

Arguments

{>> TODO <<}

Properties

There are two ways you'll commonly need to write out optional properties. One is when some items on a type are required and others are optional. In that case, you can write the type with each argument specified explicitly as being optional or not:

type ATypicalWesternName = {
  first: string;
  middle?: string;
  last: string;
}

The other case is where every property on an object is optional. (This tends to come up when you're passing in a configuration/options hash to a function.) In that case, you can use the build-in Partial<T> type, which makes every property optional:

type Options = {
  doSomething: boolean;
  withSomeValue: number;
};

export declare function takesOptions(options: Partial<Options>);

If any of the arguments are required, don't use Partial, spell it out explicitly. Alternatively, build an intersection type from the combination of required properties and optional properties.

type WesternRequiredName = {
  first: string;
  last: string;
}

type WesternOptionalName = {
  middle?: string;
}

type WesternName = WesternRequiredName & WesternOptionalName;

(This is something of a last resort, but it's there when you need it!)

Union types

Union types (described here) let you describe the either-or scenario. If a function can take either a number or a string or a boolean as an argument, you can write it like this:

declare function takesAUnion(firstArg: number | string | boolean);

Likewise, if a function returns more than one kind of things, you can write it like this:

declare function returnsAUnion(): number | string | boolean;

This is most useful for times when there is no distinct mapping between the input and output type. For example, the helpers in the ember-native-dom-helpers helpers (now living at @ember/test-helpers) all take either a string or an HTMLElement as their argument, but they always return the same kind of thing. The function signature for click, for example, is:

export function click(selector: string | Element): Promise<void>;

However, in some circumstances, a function might take more than one input type and have different output types – but each kind of input would always have the same kind of output. For that, we'll use overloading.

Overloading

With overloading, we can tell TypeScript that a single function or method has different "signatures." For example, you may recall that {>> TODO: example <<}

Generics

Structuring type definitions

The basic structure of the type definition should mirror the structure of the addon or library you're documenting. However, the layout of Ember addons imposes specific requirements that are somewhat unique. In general in TypeScript, it is a good idea to lay out more complicated definitions side-by-side with the modules they document:

src/
  index.js
  index.d.ts
  some-other-module.js
  some-other-module.d.ts

However, in the current file system layout for Ember addons, the source location and the "lookup location" where you import the files from are not the same. That is, you don't import Foo from 'an-addon/addon/services/foo';, you import Foo from 'an-addon/services/foo';. This essentially constrains us to writing all of our type definitions in a single file. (The Module Unification file layout will help with this, but it will be some time before most addons are using it.)

Aside: The Ember types, as they stand today, are not a good example for how to document most libraries: we have documented Ember as it exists, where you can import things both in the new modules API and via the old global. As Ember itself moves to using the module structure under the hood and those packages are actually installed on the file system, type definitions should move to mirror them.

For a simple addon you can just put the type definitions in the root at index.d.ts, like this:

the-addon/
  addon/
    index.js
  app/
  index.d.ts
  index.js
  package.json

A prime example of this kind of setup is ember-test-friendly-error-handler.

{>> TODO: example addon for multiple modules in index.d.ts – ember-qunit maybe? <<}

{>> TODO: structure – modules etc. <<}

Adding type definitions to an addon

[Reminder: only do this after you've checked with the author(s) of the addon that they're interested!]

Once you have a version of the types working solidly in your own application(s), you can open a pull request to the addon with those types. The pull request should include:

  • the index.d.ts file in the root of the repository
  • the "types" key in package.json pointing to the file – for the sake of forwards-compatibility, so that if the type definitions are expanded-and-moved at a later time, things don't surprisingly break
  • a note in the README that type definitions are supplied with the addon

It should also have a good writeup, describing any open questions, pain points, limitations, etc. – that will help the addon maintainer and the reviewers help you wrap everything up with a bow!

Your best bet is to solicit feedback from both the addon maintainer and from knowledgeable folks in #topic-typescript on the Ember Community Slack. The Typed Ember team, currently consisting of @dfreeman, @dwickern, @jamescdavis, and @chriskrycho, are happy to help review these requests, and over time others will be experienced enough to chip in as well!

If you get feedback that things need tweaking, don't worry! Type definitions are hard to get just right, and the definitions for Ember have been through a ton of revisions and we're constantly finding issues with and helping each other with those, too!

Publication

You should also make sure the type definition is not excluded by .npmignore file and if the package.json is manually specifying files via the "files" key, that index.d.ts is in the list. (This is unusual for Ember addons.) See the documentation for the files key for details.

Adding type definitions to DefinitelyTyped

{>> TODO <<}

DefinitelyTyped is an officially supported, Microsoft-owned GitHub repository which publishes to the @types npm namespace. Type definitions are automatically published from each folder in the types directory in the repository.

The process when using DefinitelyTyped is mostly the same as when adding them directly to an addon. When opening a PR for a set of types on DefinitelyTyped, as when doing it directly to an addon, you should get reviews from other folks who are comfortable with both TypeScript and the addon. The Typed Ember team, currently consisting of @dfreeman, @dwickern, @jamescdavis, and @chriskrycho, are happy to help review these requests, and over time others will be experienced enough to chip in as well!

Once it's in place, if the addon author is up for it, you can then open a PR to add a note to their README indicating that types are available at @types/<the-addon-name>. You should also open a PR to this repository to update the known-typings.md file.

Typings to contribute

I’ve broken this down into two categories: Top 100 Addons on Ember Observer and Other addons. Both of these are important, albeit for different reasons.

The top 100 addons on Ember Observer represent the most-installed addons in the community; their importance is probably obvious—getting typings in place for them immediately impacts the most people. Other addons or packages are those which may be downloaded less, but whose authors actively want to participate in the quest! We want to support both of these.

Top 100 Addons on Ember Observer

The top 100 add-ons in the ecosystem, by way of Ember Observer! Some of these may not need typings added; we’ll remove them as we evaluate that. There is also quite a range of variation in the complexity of these.

Other addons

If your addon isn't in the top 100 on Ember Observer but you volunteer it, we'll list it here!

@chriskrycho chriskrycho added the types:core Something is wrong with the Ember type definitions label Jul 27, 2017
@kratiahuja
Copy link

@chriskrycho what's the story on linting with typescript? Will this addon support tslint or does eslint work well right now?

@chriskrycho
Copy link
Member Author

@kratiahuja the short version is: this add-on leaves that to the user entirely, but we will (for the moment) recommend TSLint, because the TS support in ESLint isn't mature enough yet.

@kratiahuja
Copy link

@chriskrycho awesome! thanks for the super quick response. Appreciate it!

@dwickern
Copy link
Contributor

dwickern commented Aug 2, 2017

It would be nice to document what steps an addon author should take to add typings for their addon.

@chriskrycho
Copy link
Member Author

chriskrycho commented Aug 2, 2017

@dwickern that's a great idea and I'm planning to include it in the writeup. (I'd also like to land #47 first so that authors who are actually writing TS can do that automagically.)

@chriskrycho
Copy link
Member Author

chriskrycho commented Aug 23, 2017

Closing in favor of typed-ember/ember-typings#14, since we're driving Ember type definition development through that respository.

@chriskrycho
Copy link
Member Author

Reopening since we've deprecated typed-ember/ember-typings. I've copied the body of the quest. Important comments from that thread:

@sdhawley (comment):

Has any work been done on ember-simple-auth? I'm getting complaints when building:

app/controllers/application.ts(3,28): error TS2307: Cannot find module 'ember-simple-auth/services/session'.

and (comment):

I've started creating an ember-simple-auth.d.ts locally as I run into things, happy to share it.

@championswimmer (comment):

Regarding ember-moment I think we do not need typedefinitions. Mostly it is hbs helpers. The Javascript API is same as moment itself, which ships with typedefinitions itself.

@denzo (comment):

https://github.com/dockyard/ember-one-way-controls is deprecated

@chriskrycho chriskrycho reopened this Jul 6, 2018
@chriskrycho chriskrycho changed the title Typings! – a quest issue [DRAFT!!!] QUEST: Let's type the Ember.js ecosystem! Jul 6, 2018
@chriskrycho chriskrycho added enhancement help wanted deferred We have a specific reason for delaying implementation of this (but do plan to!) labels Jul 6, 2018
@AvremelM
Copy link

Is this doc intended to cover contributing to core typings (e.g., @ember/*)? Is there anywhere else where that would be documented?

@chriskrycho
Copy link
Member Author

chriskrycho commented Oct 18, 2018

No: those types all already exist and are largely complete! (We are of course continuing to tighten them up as we find issues or think of better ways to express things.) They live in the DefinitelyTyped repo.

@AvremelM
Copy link

Sorry, I meant contributing to those existing typings. For example, correcting issues like API changes or types that match the Ember API docs, but where those docs are incomplete/incorrect. E.g., @ember/template is not in DefinitelyTyped at all.

Should I even be trying to contribute to those, or just reporting issues and letting the experts handle those?

@chriskrycho
Copy link
Member Author

We’d welcome help! We mostly want to make more experts, not hoard expertise. Please open bugs on this repo (there’s a template for type definition bugs you can choose when you click the Create Issue button!). We’re very happy to mentor people through contributing fixes for them.

@mike-north
Copy link
Contributor

@HodofHod thanks for your desire to jump in and help!

E.g., @ember/template is not in DefinitelyTyped at all.

At the highest level, we want to ensure that the type information tells the truth about Ember's public API. In the case of this package, it's not represented anywhere in the docs, so it should not be in the types.

screen shot 2018-10-18 at 10 11 21 am

If you believe that this is part of Ember's public API, the first step would be to correct the documentation. Only then can we use an authorized documentation change as an idication of "this is public", and then add it to types.

If you're not familiar with the best practices for writing and testing declaration files, I have a blog article here that may be helpful https://medium.com/@mikenorth/guide-to-typescript-ambient-declarations-717ef6da6514

@AvremelM
Copy link

AvremelM commented Oct 18, 2018

Hey, I just (already) created #342

You're correct that it doesn't appear in the sidebar, but it actually is listed in the docs for @ember/string, so I'm not sure about editing the API docs.
image

That makes a lot of sense regarding making sure the types match the docs, I'm just not sure about this case

@mike-north
Copy link
Contributor

mike-north commented Oct 18, 2018

I would raise this with the learning team. The code documentation tools they use require some non-trivial finesse to fix this kind of thing.

As far as the types go, because this is a "synthetic" package that's not really in the /packages folder of the ember monorepo, it should be done via a module declaration within the @types/ember__string source.

Also, for there to be any value in having a @ember/template package, developers should actually be importing stuff from it. The individual functions isHtmlSafe and htmlSafe sugest that these functions should be imported from @ember/string anyway.

screen shot 2018-10-18 at 10 42 08 am

in the future this may change, but we may want to wait until a beta is cut with the deprecation before leading people in this direction.

At least for the ember 3.x release series, we need to also maintain the existing means of importing htmlSafe and isHTMLSafe

@AvremelM
Copy link

AvremelM commented Oct 18, 2018

That makes sense. I'd just add that based on this I'd say that @ember/template is already the "correct" location to import from, though of course @ember/string should continue to work. (Edit: interestingly, it seems that at the time, importing from @ember/template wasn't working at all, though it definitely works for me on 3.3)

The ember-modules-codemod is already making this change for people, which is how I stumbled on this.

@Panman82
Copy link

Panman82 commented Dec 9, 2021

Been a while, any movement here? Maybe there is another direction that I'm not aware of.?. Thanks!

@chriskrycho
Copy link
Member Author

No, and we should probably rethink this whole approach. I am closing (and unpinning) this issue and folks should feel free to approach types organically. Typed Ember folks are of course happy to provide guidance and recommendations, though!

@chriskrycho chriskrycho unpinned this issue Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deferred We have a specific reason for delaying implementation of this (but do plan to!) enhancement help wanted types:core Something is wrong with the Ember type definitions
Projects
None yet
Development

No branches or pull requests

6 participants