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

Synchronous dynamic module loading? #19495

Closed
jez9999 opened this issue Oct 26, 2017 · 14 comments
Closed

Synchronous dynamic module loading? #19495

jez9999 opened this issue Oct 26, 2017 · 14 comments
Labels
Out of Scope This idea sits outside of the TypeScript language design constraints

Comments

@jez9999
Copy link

jez9999 commented Oct 26, 2017

In Node.JS the following pattern can be used to load optional dependencies:

try {
  var foo = require('foo')
  var fooVersion = require('foo/package.json').version
} catch (er) {
  foo = null
}
if ( notGoodFooVersion(fooVersion) ) {
  foo = null
}

// .. then later in your program ..

if (foo) {
  foo.doFooThings()
}

This doesn't work with an import foo = require() because import cannot be inside a try block. Dynamic imports have to be async, which unlike the above, requires the calling code to be async. How about having synchronous dynamic module importing which would allow an import inside a try block but load synchronously?

@ghost
Copy link

ghost commented Oct 26, 2017

We're trying to follow the ES modules spec, which means only allowing import statements in places allowed by the spec.
If you just want something hacky that works now, you can call require from TypeScript code if you have @types/node installed:

import AbsType = require("abs"); // Not a runtime import
let abs: typeof AbsType | undefined;
try {
    // tslint:disable-next-line no-var-requires
    abs = require("abs");
} catch (_) {}

if (abs) {
    console.log(abs("abc"));
}

Of course this won't work in a browser.

@ghost ghost added the Out of Scope This idea sits outside of the TypeScript language design constraints label Oct 26, 2017
@kitsonk
Copy link
Contributor

kitsonk commented Oct 26, 2017

Of course this won't work in a browser.

Unless you post-process it via some sort of bundler. 😁 But yeah, synchronous module loading is the bane of browsers.

@jez9999
Copy link
Author

jez9999 commented Oct 26, 2017

@andy-ms Won't that code try to require() the optional module twice on the supported platform? Once for the import and once for the require inside the try?

@ghost
Copy link

ghost commented Oct 26, 2017

So long as you only use the first import as a type (hence the name AbsType to remind you not to use it as a value) it won't show up in the compiled code at all.

@jez9999
Copy link
Author

jez9999 commented Oct 26, 2017

Oh right I guess that works.

@NN---
Copy link

NN--- commented Oct 29, 2017

Sometimes I needed to have synchronous loading to load only required modules depending on some flags.
I solved it with defining 'require' function but I don't have any intellisense.
Is there any option to define module type same way as 'import' works ?

declare function require("./mymodule"): typeof module "./mymodule";

@ghost
Copy link

ghost commented Oct 30, 2017

@NN--- See above with the AbsType example. Just declare require to return {} | null | undefined (or install @types/node and use the definition from there) and cast to the module type.

@weswigham
Copy link
Member

@andy-ms We should consider handling const name = require("name") as gracefully (ie, strongly type the module) in TS as we do in JS. Casting to the module type feels like a bit of a kludge when 1. You could suppress an error on a nested import statement with ts-ignore and get the right types, and 2. You can do the same code in a JS file and get the right types.

@kitsonk
Copy link
Contributor

kitsonk commented Oct 31, 2017

@weswigham my only comment is that is potentially a 👣 🔫 in authoring code that is specifically not portable and won't get re-written... It feels like it is a necessity in JavaScript, but something that shouldn't be easy to do in TypeScript (or something that should be behind a flag).

@weswigham
Copy link
Member

@kitsonk Feels like a pretty small foot gun if your output format is commonjs; esp if you've already got a global require function with any types provided by the node .d.ts (which is a much larger foot gun).

@kitsonk
Copy link
Contributor

kitsonk commented Oct 31, 2017

Fair point... I guess module format commonjs is enough of a flag.

@ghost
Copy link

ghost commented Oct 31, 2017

There is a lint rule to remind people to use typed import instead of untyped require.

You could suppress an error on a nested import statement with ts-ignore and get the right types

Right, that is probably a better solution.

@weswigham
Copy link
Member

You could suppress an error on a nested import statement with ts-ignore and get the right types
Right, that is probably a better solution.

I said you get the right types... I believe the emit is broken because we only seek to transform top-level import statements (we just have no such optimization in the type checker).

And again, if the node .d.ts is including such a "foot gun" as an untyped import because people still use the pattern (and will for the foreseeable future, because commonjs node modules aren't going anywhere anytime soon, even as native esm gains support, since the interop story is so mediocre), we should meet users where they are and support idiomatic patterns for their target, just like we do in JS. And if you're looking for the ability to swap your module target to esm 5 years from now, import name = require("name") is just as broken as const name = require("name"), so you shouldn't be using either - saying it's not a future proof pattern and to use the other isn't really a cohesive argument anymore given our decisions leading to this point. Originally we didn't support this because we didn't want to do the static analysis ("people could alias require" and so on), but it turns out supporting the simple usecases like in our JS analysis covers the 99% of real uses (and falling back to an untyped require with optional implicit any if not statically analyzable covers the rest); but now we have the machinery to do it for JS users, and we're shafting TS users for what? An inconsistent pragmatism toward the es6 spec? We see issues like this one opened with frequency and the stackoverflow answers explaining the workaround are popular.

That's a good indicator we should change our behavior to accommodate our users since we no longer have any technical barriers to doing so.

@typescript-bot
Copy link
Collaborator

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Out of Scope This idea sits outside of the TypeScript language design constraints
Projects
None yet
Development

No branches or pull requests

5 participants