TypeScript declarations: incorrect Interceptor method signatures

[atsu85]

Generated TypeScript declarations for Interceptor:

http-client/dist/aurelia-http-client.d.ts

Lines 76 to 91 in 6a3f44a

	response?: Function;
	/**
	* Intercepts a response error.
	*/
	responseError?: Function;
	/**
	* Intercepts the request.
	*/
	request?: Function;
	/**
	* Intercepts a request error.
	*/
	requestError?: Function;

...have neither parameters nor return types.

Official documentation provides some information about parameters and return types, but I think it is not accurate either.

5 months ago i came up with following type declaration for Interceptor (that fulfilled requirements for my code):

    export interface Interceptor {
        request?(message: HttpRequestMessage): HttpRequestMessage | Thenable<HttpRequestMessage>;
        // TODO types?
        requestError?(error);
        response?(message: HttpResponseMessage): HttpResponseMessage | Thenable<HttpResponseMessage>;
        /** throw responseError to reject or return some HttpResponseMessage to recover from the response error */
        responseError?(responseError: HttpResponseMessage): HttpResponseMessage | Thenable<HttpResponseMessage>;
    }

    /**
     * Part of the Promise that only has then function
     */
    interface Thenable<T> {
        /**
        * Attaches callbacks for the resolution and/or rejection of the Promise.
        * @param onfulfilled The callback to execute when the Promise is resolved.
        * @param onrejected The callback to execute when the Promise is rejected.
        * @returns A Promise for the completion of which ever callback is executed.
        */
        then<TResult>(onfulfilled?: (value: T) => TResult | Promise<TResult>, onrejected?: (reason: any) => TResult | Promise<TResult>): Promise<TResult>;
    }

...but I haven't investigated the source after that. One thing that should probably be changed is replacing HttpRequestMessage with RequestMessage and smth similar with HttpResponseMessage.

---

I'm really sad/disappointed that Aurelia team decided in favor to ES6 with type info instead of writing the code in TypeScript. In theory everything should be more-less OK with TypeScript declarations, but in reality almost all Aurelia subprojects have either no TypeScript declaration info or there isn't much to do with automatically generated declarations if You want to use type info in webapp TypeScript source for Aurelia objects (and i'm not even talking about --noImplicitAny TypeScript compiler flag).

Please, don't get me wrong, i absolutely love Aurelia and very much appreciate the effort from Aurelia team and other contributors, but this approach has many drawbacks (and no significant benefits, if any, that i know about):

1. It produces inaccurate and incomplete TypeScript declarations.
2. It makes it more difficult to understand the source code.
3. Helping You with TypeScript declarations info is several times more time-consuming and error-prone compared to just adding TypeScript declaration info incrementally after starting to use TypeScript 1.6 with TypeScript compiler.
4. Helping You with source code is also much more time-consuming:
   a) IDE support for TypeScript helps to understand code much faster
   b) IDE support for TypeScript helps to write fixes and new code much faster
   c) TypeScript compiler checks trivial errors
   ...i guess i could continue, but i have to run...

Is there anything missing from TypeScript 1.6 that You'd like to use in Aurelia source code? I can't recall any ES6/ES7 features missing from TypeScript 1.6 (see https://github.com/Microsoft/TypeScript/wiki/Roadmap) that I've seen in aurelia source code. @cmichaelgraham can You think of any?

:(

[EisenbergEffect]

There are a number of issues with us writing Aurelia in TypeScript at present:

1. I haven't seen anyone demonstrate cross-repo dependencies working correctly with TS builds. I've been asking for months for anyone on my own team or the TS team to show me multiple separate GitHub repos, that are independently released, written in TS, with dependencies between the repos, and a working build. It also needs to be possible to build dependent repos against updated libraries without releasing. It may be possible, but I've never seen or heard of this being done, and it's critical for Aurelia, since we are so modular.
2. Related to 1, I'm not sure the TS compiler configuration options are advanced enough to properly resolve all modules at compile time. There was some work on that, but I'm not sure if it's been released and if it has, how to use it.
3. There are features in TypeScript which are likely to conflict with ES 2016. That means that if we switch, we have to maintain strict use of TS features so that we don't end up with a codebase that is incompatible with the future direction of the web. For example, we couldn't allow the use or private or protected.
4. There is a significant percentage of our community that hates TypeScript and won't use Aurelia if it's written in TypeScript.
5. I've built open source in the past on top of Microsoft technologies...and have been severely burned by that more than once. I hope TS is different, but my experiences make me very hesitant to bank our entire product and company future on TypeScript.
6. TypeScript has a much slower release cycle than Babel, so it gets new standards-based features and bug fixes slower. With Babel, I can literally message the project organizers about a bug and they will usually have a fix out within the same day...sometimes within the hour. With TypeScript we might have to wait months.
7. One of the big selling points of Aurelia is that it is so standards-based. Switching to TypeScript would mean that we couldn't make that claim as strongly.
8. Existing intellisense tooling can't really handle Aurelia's codebase, as a whole, very well, so I'm not sure that having it in TS would solve that problem. The experimentation I've done in this area have looked pretty grim.
9. Involving the TS compiler for the initial builds would cause us to create more dependencies between modules and libraries than are necessary. In a number of cases, this would create circular dependencies. Because TS can't tell the difference between the type that was imported strictly for typing purposes and one that was imported for runtime use, all types have to be pushed into the generated TS5 code as module imports. This can result in performance degradation due to unnecessary loads as well as runtime circular dependencies between modules.

Please don't get me wrong, I like TypeScript a lot and think it's probably a good fit for most applications. But I'm not sure it's the best fit for Aurelia's sourcecode base...at least not yet I'm open to moving to TS in the future, but I'm just not sure TS is ready for us yet.

@atsu85 Please submit a PR to fix up the interceptor signatures. Or maybe @bryanrsmith can add those? Your'll find that a lot of our types are more filled out in master now. And we're planning to have them completed for the beta in the next week or so. If you find stuff that is missing or incorrect, please help us by submitting PR's.

[atsu85]

Hi @EisenbergEffect,
Sorry for the late reply. Probably discussion about concerns of adopting TypeScript should be listed under separate issue, but I'll continue the discussion here.

Regarding Your concerns with adopting TypeScript (TS from now on in this comment), I'll use the same numbers for 9 concerns You had:

1. By compiling TS to ES6 "cross-repo dependencies" isn't a concern build process can remain (pretty-much) the same after compiling TS to ES6 (but it might make sense to put generated ES6 files to some other location).
2. Based on documentation TS is able to emit 'commonjs', 'amd', 'system', and 'umd' module formats and ES6 with (--target ES6), but even if some of them have bugs, build process could be left mostly the same after compiling TS to ES6 (using --target ES6) and after that with existing tasks from ES6 to different module formats.
3. Could You mention some more TS features in addition to private/protected modifiers, that concern You regarding potential conflicts (with TS and ES7 i assume)?

For example, we couldn't allow the use or private or protected.

This doesn't concern me at all - one obvious option would be not to use private and protected modifiers at all, but the other option that seems more reasonable is to deal with this problem when (if at all) it occurs - find&remove all usages of private and protected modifiers is simple enough.
4. I don't see why someone should refuse to use Aurelia when Aurelia code is written in TS. It isn't like they would be forced to write TS themselves to use Aurelia. I'm assuming it is just a blind hate caused by the ignorance - code with exactly the same syntax as used right now (that already includes type info). Just currently build process doesn't check if the code makes any sense regarding documented type info and actual usage. Starting to use TS helps verify that it is written without obvious errors that compiler can detect. In addition not all TS type info can be compiled to proper d.ts with Babel TS plugin (probably smth else i'm not aware of, but things that have made me worry the most: 1. generics. 2. argument types for function that is used as a function argument).
5. Can i assume that those MS technologies that have burned You are not OS? You can see, that TS repo contains lots of accepted PRs from non-MS contributors. Also if smth starts to take wrong direction, Aurelia has 2 potential solutions to come out of it: 1) start using ES6 again - either that is produced by TS compiler or just rename js files and see what needs to be done to make it acceptable again for Babel TS plugin or some other similar tool 2) fork TS, as it happened with node and io and hope it turns out just as fantastic as it turned out for these two projects (io being recently merged back to node).
6. I have to agree that TS release cycle was disappointingly slow up to version 1.5 - they have improved it a lot by now (shorter release cycles because of smaller feature sets). On the positive side they don't rush into changes that may hurt Aurelia (and other projects) as much as Babel's recklessboisterous changes. So You might take it as an advantage instead of disadvantage :)
7. Starting to use TS to produce ES6 doesn't make the produced ES6 any less standards based. Additionally Aurelia already relies on not only ES6 standards, but also ES7 proposals that are subject to change. Also TS is moving quickly to become completely compatible with ES6 (TS version 2.0 is supposed to be completely ES6 compatible).
8. Can You clarify how adopting TS would make intellisense tooling any worse than it is now - I'd expect the opposite.
9. If i understood correctly what You tried to say, then there is no problem with this point either.

Because TS can't tell the difference between the type that was imported strictly for typing purposes and one that was imported for runtime use

This is wrong assumption - at least with TS 1.6.2 and i think it was the same way with 1.4 as well (if not always). So this is not a problem. If i import interface and class from the same file, then in the compiled output only class is imported from that file. TypeScript compiler understands that Interface doesn't have runtime representation, so it doesn't make sense to emit it as an import (it uses imported interface just for type checking).

In a number of cases, this would create circular dependencies.

This isn't problem either - Java/C# devs know how to resolve circular dependencies and this isn't different for TypeScript either.

I hope i haven't misunderstood smth, but if i have, please correct me - I'd love to know. Also it would be nice, if some other TypeScript users could verify my statements (I've seen some activity/interest for TS from @cmichaelgraham, @ctoran, @jdanyow )

[EisenbergEffect]

@bryanrsmith When you get a chance, can you update the interfaces for interceptors to the correct signatures? I'm not as familiar with that part of the codebase or that feature, so I'm not sure I'd get it exactly correct.

[atsu85]

Regarding http-client type declarations:

I'm not as familiar with that part of the codebase or that feature, so I'm not sure I'd get it exactly correct.

I can only say the same about myself, so I'd also appreciate help from @bryanrsmith

[atsu85]

@EisenbergEffect, Regarding TypeScript, do you have any comments? Maybe You could ask someone from the Aurelia core team to look into Your concerns and my answers about TypeScript that we listed above?

[ctoran]

I agree with @atsu85 that when compiling TypeScript to ES6 (and not ES5), most of the concerns @EisenbergEffect mentioned above will become non-issues. Remember that TS is only a type checking layer on top of your JavaScript code and will output what you are already writing by hand in ES6. At runtime the browser's engine sees the same thing but what you get is some magical tooling.

[EisenbergEffect]

It would be great if someone sent a PR to update the interface definitions we have...

[vuorinem]

Are the current type definitions correct? I have an interceptor with responseError handler that used to take HttpResponseMessage as a parameter, but after this change the typings suggest that the handler takes Error parameter.

Looking at the code, I would assume that the parameter is HttpResponseMessage for both response and responseError handlers. This is also what @atsu85 has in the first comment.

[EisenbergEffect]

Can you submit a PR to fix that?

[vuorinem]

Did my best trying to figure out how the promise chain works in RequestMessageProcessor. Does that look right?