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

Feature request: better support for typing async imports #11611

Closed
dgoldstein0 opened this issue Oct 14, 2016 · 15 comments
Closed

Feature request: better support for typing async imports #11611

dgoldstein0 opened this issue Oct 14, 2016 · 15 comments
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript

Comments

@dgoldstein0
Copy link

Currently, typescript has great support for ES6 style imports - e.g.

import * as $ from "jquery";

function doSomething(): void {
  $("#foo")...
}

typescript is able to know that "jquery" here is whatever defines the jquery module, and $ gets the type associated with jquery's exports.

However, if you try to async load code in typescript, it tends not to know the types. E.g. in AMD, the following code would make sense:

function doSomethingAsync(): void {
  require(["jquery"], function($) {
    $("#foo")...
  });
}

but this leaves $ with the type any (and noImplicitAny would complain about this). The options to handle this right now are:

  1. use definition files. Annoying because we end up with definition files for pure typescript, which presumably already declare all their types; it's a lot of unnecessary overhead.
  2. leave code untyped / make the any explicit.
  3. duplicate the types all over the place - e.g. here, declare the jquery type inline. This really isn't maintainable.
  4. don't do async imports. This is not really an option for large applications that care about performance.

While I've describe the situation for AMD, my understanding is that other forms of async module loading (such as conditional require() calls in commonjs) suffer the same problems.

note: unlike #3100 I am neither suggesting new syntax, or that typescript should handle loading async code in any way; I would just like to see it be able to strongly type such code as it already handles ES6-style imports, and leave the implementation details of module loading to actual module loaders like node.js or requirejs.

@aluanhaddad
Copy link
Contributor

function doSomethingAsync(): void {
    let x = require(['jquery'], function ($) {
        $('#foo');
    });
}
declare function require(modules: ['jquery'], f: ($: JQueryStatic) => void);

Is not that much extra code to maintain. Module import syntax and semantics need to converge instead of having more variants supported by more tools. There are proposals to add new forms of imperative imports to ECMAScript. If that happens I imagine TypeScript would transpile these into corresponding syntax for the target module format.

@dgoldstein0
Copy link
Author

dgoldstein0 commented Oct 14, 2016

@aluanhaddad but where does the JQueryStatic symbol come from? I think it'd have to be a definition file.

And maybe jquery wasn't the best example. Replace it with some pure typescript module, written with ES6 imports. Importing it with require now needs a whole type definition file - for something that didn't need a definition file before? seems like a lot of added friction.

Module import syntax and semantics need to converge instead of having more variants supported by more tools

If by this you mean we should have a single way to do async imports, then sure, I can jump on board with that; but the fact remains there is no "es6+ async import" spec. But as someone who writes code that targets the browser, we need a solution that can do both sync and async imports; wishing away async imports may work server side, but it's not a rational choice in the browser.

@aluanhaddad
Copy link
Contributor

JQueryStatic comes from a file e.g. jquery.d.ts that you downloaded or installed via a package manager. It is the same file which provides the $ declarations which you use in non async import contexts. Of course that brings us directly to your second point.

And maybe jquery wasn't the best example. Replace it with some pure typescript module, written with ES6 imports. Importing it with require now needs a whole type definition file - for something that didn't need a definition file before? seems like a lot of added friction.

If the TypeScript file has exports we can use them just like we would from a declaration file. Even if it were a declaration file, TypeScript would retain imports for the corresponding module if we use any values from it in value position.

I agree there is an issue here but it is not really a TypeScript issue as much as an issue with ES Modules. ES Modules couple declaration and definition very heavily. I don't have a good solution.

@aluanhaddad
Copy link
Contributor

By the way there are some proposals you may find of interest.
Nested import declarations

import()

@dgoldstein0
Copy link
Author

If the TypeScript file has exports we can use them just like we would from a declaration file. Even if it were a declaration file, TypeScript would retain imports for the corresponding module if we use any values from it in value position.

I'm not totally clear what you mean by this. Probably it'll be more clear with an example: let's suppose I have a file foo.ts which compiles to module foo that I want to async load:

foo.ts:

export function blah(a: A, b:B) : void {
  ...
}
export function blahblah(c: C, d: D): E {
  ...
}

And then within bar.ts I want to do something like:

require(["foo"], (foo) => {
  ...
})

Now I can manually type foo as {blah: (a: A, b: b) => void, blahblah: (c: C, d: D) => E} but this is ugly because now I'm completely re-specifying everything in foo, with no real check that this definition is accurate. Also, unlike the jquery case, there's no existing JQueryStatic type I could reuse here. I suppose I could write a definition file for foo.ts, but this is a lot of redundant code. Ideally, foo.ts should serve as it's own definition file (not sure if that's a thing yet?)

Anyhow, to be totally clear: I'm hoping for a solution soonish - probably before ES6 settles on any standardized way of doing things. Fwiw, I looked over those two proposals - nested imports doesn't seem to address how to do them asynchronously, but import() and the await import idea discussed in it both sound like they could solve the problem. But anyhow I don't have a great grasp on what the timeline for those proposals getting standardized / implemented is, so it seems like we should figure out how to make our current abstractions work?

With only minor additions of syntax to typescript, I guess what I would like to see would be something like this:

require(["foo", "bar"], function(foo: TypeOfModule("foo"), bar: TypeOfModule("bar")) {
  ...
});

Where we'd add this TypeOfModule builtin (open to better naming suggestions) that would take a string literal and return the type of the module it corresponds to - so TypeOfModule("foo") would result in the same type as Foo in import * as Foo from "foo";

Now I'll admit that I don't love what I'm suggesting because it means we end up duplicating our module names, and need to also check that the types of the arguments to the callback match the order of the module names in require's first arg; but this can be statically verified with TSLint or something similar. This proposal seems to be the right tradeoff between "practical" (probably not hard to implement) and "doesn't suck to maintain code written like this" - though long term I'd hope for something better.

To do an even better job on AMD require, we'd need a way to express the constraint that if require's first argument (deps) is an array of string literals, the second argument should be a callback with type (...args: *deps.map(TypeOfModule)) => void where roughly the * means python-style tuple unpacking (so our variadic function can have specific types on each argument) and deps somehow magically references the first argument of require. I suppose we could throw away the variadic-abilities and only support async loading a fixed number of modules at a time, but we still have the matter than even in the simple case of require(["a"], (a) => ...) the type of the second arg to require depends on the value of the first arg - which I believe isn't currently expressible in typescript, at least not without having special cases for every actual invocation. But this all seems like a lot of complexity to add when an actual standardized async import spec looks to be coming within the next year.

@mhegazy mhegazy added the Suggestion An idea for TypeScript label Oct 17, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Oct 17, 2016

I would add that implementing this would not be as simple as that, since it probably needs to work as a type operator, e.g.

load(moduleName:string): shapeof moduleName;

moreover, anything we do here will need to respect the module resolution logic we have today, and other commandline options paths, baseUrl, rootDirs, etc..

it is worth noting that given how the compiler is architected, this is not a trivial change.

@mhegazy mhegazy added the Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. label Oct 17, 2016
@dgoldstein0
Copy link
Author

sure, that's totally understandable - I'd definitely want it to use the
same module-resolution logic that already exists.

In my opinion it's a worthwhile change, since it fills a real need we have
to make types work well with module loading systems that aren't es6 imports

  • mainly since es6 imports don't yet solve all of our problems. One day
    hopefully they will, but if we can have some intermediate ground sooner it
    would be much appreciated.

On Mon, Oct 17, 2016 at 2:41 PM Mohamed Hegazy notifications@github.com
wrote:

I would add that implementing this would not be as simple as that, since
it probably needs to work as a type operator, e.g.

load(moduleName:string): shapeof moduleName;

moreover, anything we do here will need to respect the module resolution
logic we have today, and other commandline options paths, baseUrl,
rootDirs, etc..

it is worth noting that given how the compiler is architected, this is not
a trivial change.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#11611 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABBnd8zsuIrmgOW6LQRp0X8YY__dwWjuks5q0-tpgaJpZM4KWonR
.

@mhegazy
Copy link
Contributor

mhegazy commented Oct 18, 2016

Worth noting that there is a related proposal in TC39 for asynchronus imports, see https://github.com/domenic/proposal-dynamic-import.

@dgoldstein0
Copy link
Author

wouldn't implementing the TC also require a lot of this work to happen? what I'm hearing is that this isn't trivial because the module resolution code isn't properly abstracted / easy to use in another place; and any ES7+ async imports deal will have the same challenge during implementation, right?

@mhegazy
Copy link
Contributor

mhegazy commented Oct 20, 2016

yes. I am just adding a related topic.

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Oct 23, 2016

@dgoldstein0 I am quite curious how often you run into this problem. I have spent a lot of time working on TypeScript + AMD and TypeScript + SystemJS projects. The only times when I found myself needing to write

require(['dep1', 'dep2', 'depN'], (dep1, dep2, depN) => {
  ...
});

or

Promise.all(['dep1', 'dep2','dep3'].map(dep => SystemJS.import(dep)))
  .then(([dep1, dep2, dep3]) => {
    ...
  });

were to load very specific, very large bundles. This was fairly rare (~3-4 times in a medium sized app) so the burden of manually typing them was not significant.
I am not saying my experience is representative, but I am curious how often this comes up.

@dgoldstein0
Copy link
Author

That's a good question. The general answer though is not enough - we have
a fairly large amd codebase - 2500 files, and I think over 1500 excluding
tests - and we have a rather large performance problem, which we are hoping
to solve by async loading more code. So the answer is that we probably
have a few dozen async imports in our codebase, but I'm trying to push for
having a lot more, which is part of why I want to reduce friction to
writing them.

On Sun, Oct 23, 2016, 11:27 AM Aluan Haddad notifications@github.com
wrote:

@dgoldstein0 https://github.com/dgoldstein0 I really curious how often
you run into this problem. I have spent a lot of time working on TypeScript

  • AMD and TypeScript + SystemJS projects. The only times when I found
    myself needing to write

require(['dep1', 'dep2', 'depN'], (dep1, dep2, depN) => {
...
});

or

Promise.all(['dep1', 'dep2','dep3'].map(dep => SystemJS.import(dep)))
.then(([dep1, dep2, dep3]) => {
...
});

were to load very specific, very large bundles. This was fairly rare (~3-4
times in a medium sized app) so the burden of manually typing them was not
significant.
I am not saying my experience is representative, but I am curious how
often this comes up.


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#11611 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABBndxoQnIaTr8ff4ri4VCkR58udZ2pJks5q26b3gaJpZM4KWonR
.

@aluanhaddad
Copy link
Contributor

More than two dozen explicit dynamic imports would a be a lot of XHRs from a production point of view.

I don't think your approach will improve performance during development if you are loading single files since the the api calls in the emitted JavaScript are already async on a per module dependency basis and get cached by RequireJS. This is really something that helps with bundles, otherwise, I believe you will just make your life harder. I say this especially because require returns void so you will end up using more callbacks and so on.

@dgoldstein0
Copy link
Author

We are doing bundling too, and use a bunch of tricks parallelize module
loading too. I'm extremely aware of AMD semantics.

For some context: The idea of our async loading is going to be for code
that's not immediately needed, and often never used. Like for modals that
most users don't view on an average page load. Currently these are almost
all synchronous dependencies so slow down time to initial interactivity,
which is extremely undesirable.

Anyhow as we do more async loading we'll definitely figure out more
optimizations to employ. The request here is just to make it easy to async
load without having to jump through too many hoops to get the types correct.

On Wed, Oct 26, 2016, 2:55 AM Aluan Haddad notifications@github.com wrote:

More than two dozen explicit dynamic imports would a be a lot of XHRs from
a production point of view.

I don't think your approach will improve performance during development if
you are loading single files since the the api calls in the emitted
JavaScript are already async on a per module dependency basis and get
cached by RequireJS. This is really something that helps with bundles,
otherwise, I believe you will just make your life harder. I say this
especially because require returns void so you will end up using more
callbacks and so on.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11611 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABBnd_YaDGy20O12vLcQroORiI44PBvhks5q3yOHgaJpZM4KWonR
.

@dgoldstein0
Copy link
Author

I believe the await import() support in typescript 2.4 solves this problem. Haven't gotten around to using it yet (still on typescript 2.3 at the moment) but I'll reopen this issue if it doesn't.

@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
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants