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

Support shorthand ambient module declarations and wildcard chars in module names #6615

Closed
mhegazy opened this issue Jan 25, 2016 · 74 comments
Closed
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@mhegazy
Copy link
Contributor

mhegazy commented Jan 25, 2016

follow up on #6614

Related issues: #247 (comment), #5787, #2709, #6371 (comment)

Problems:

  • Can not easily specify a loader extension for json or css (e.g. `json!a.json); currently every resources needs to have its own declaration
  • Problem with migrating to .ts, with modules that do not have typings, and users just want to get by

Proposal:

  • Allow for short hand module declarations:

    declare module "foo";

    to be equivalent to:

    declare module "foo" {
        var _temp: any;
        export = _temp;
    }
  • Allow module names to have a wildcard character and match that in lookup

    declare module "json!*" {
        let json: any;
        export default json;
    }
    
    import d from "json!a/b/bar.json";
    // lookup:
    //    json!a/b/bar.json
    //    json!*
  • Additional the module "*" would allow for matching all unknown modules in the system, which can be used as a way to suppress all missing module errors

    declare module "*";
    
    import d from "some\unknown\module"; // no error, d is any
  • Report no-implicit-any errors for modules declared using the shorthand notion
    Open issue: is this reported on declaration or use sites

@mhegazy mhegazy added Suggestion An idea for TypeScript Committed The team has roadmapped this issue labels Jan 25, 2016
@mhegazy mhegazy added this to the TypeScript 2.0 milestone Jan 25, 2016
@mhegazy mhegazy self-assigned this Jan 25, 2016
@mkosieradzki
Copy link

Epic solution for #2709 !

@dfaivre
Copy link

dfaivre commented Jan 25, 2016

Nice to see this is being worked on -- looks like an interesting solution that neatly captures all the outstanding requests.

Personally, I'd also like to see a compiler flag for the declare module "*"; use case -- it seems a little more natural, something like noExplicitAny, except for modules.

Thanks again for all the great work!

@nillis
Copy link

nillis commented Jan 25, 2016

+1
Great solution! Looking forward to be able to delete all modules I created manually to keep the compiler happy

@ivogabe
Copy link
Contributor

ivogabe commented Jan 25, 2016

Allow for short hand module declarations:

declare module "foo";

to be equivalent to:

declare module "foo" {
    var _temp: any;
    export = _temp;
}

I think that this doesn't cover all cases, as this fails currently:

declare module "foo" {
    var _tmp: any;
    export = _tmp;
}
declare module "bar" {
    import { x } from "foo"; // Error: Module foo has no exported member x
    import y from "foo"; // Error: Module foo has no default export
}

@mhegazy
Copy link
Contributor Author

mhegazy commented Jan 25, 2016

thanks @ivogabe. the idea is you can use this module in any way you want and you just get any with no error. So the proposal should be updated to:

declare module "foo" {
    var _temp: any;
    export = _temp;
    export default _temp;
}

@amcdnl
Copy link

amcdnl commented Jan 26, 2016

I like the idea, but I think 2.0 is too far for this. This can help to solve so many issues the community has with lack of typings/etc.

@kitsonk
Copy link
Contributor

kitsonk commented Jan 26, 2016

Thanks, this certainly address my use cases!

@amcdnl, 2.0 is the next release after what is currently being stabilised as feature complete.

@geirsagberg
Copy link

This is great! (GitHub really needs an upvote button.)

@jack4it
Copy link

jack4it commented Jan 26, 2016

Two questions:

  • Are the current proposals able to cover the following scenario?
import "xyz.less";

Can I do this?

declare module "xyz.less";
  • Will this syntax be one of the proposals?
import template: string from "template.html";

@kitsonk
Copy link
Contributor

kitsonk commented Jan 26, 2016

@jack4it if I understand the proposal correct:

declare module "xyz.less";

Would automagically make the import xyz from 'xyz.less' or import * as xyz from 'xyz.less' as type any. If you are just importing for side effects, like in your first statement, it is entirely up to what module loader you are using and how it is configured. Almost any module loader will require some sort of plugin (though obviously using NodeJS's registry can absolve people from having to consider this, but that doesn't make for very "portable" code).

If you wanted the last to work, you would want to declare an ambient module like this:

declare module '*.html' {
    const template: string;
    export default template;
}

And you module loader would have to ensure that it process things appropriately.

Personally, I think any more automagic behaviour would be dangerous.

@mhegazy
Copy link
Contributor Author

mhegazy commented Jan 26, 2016

Will this syntax be one of the proposals?
import template: string from "template.html";

No. 1. is it makes it much harder to update your definitions later on as now you have sprinkled template: string throughout your code base, and 2. type annotations will need to be supported on all import forms not only default imports import d from "mod" but for namespace, and property imports, and that would add yet another variant to the import syntax, and we already have plenty.

@glen-84
Copy link

glen-84 commented Jan 27, 2016

Interesting. #3691 is also related.

I'm a little bit surprised that you're planning to support declare module "*";, after your comment here about option 2. =)

Now I'm not sure whether to use that or to keep an explicit list of shorthand module declarations. Hmm.

Edit: After reading your comments in #247, I think that I'll go with the latter option, so that if I mistype a module name, it will be reported as missing as opposed to just being typed as any.

@olostan
Copy link

olostan commented Feb 15, 2016

For those who seek for quick solution until 2.0 will be released: I've added line

        if (moduleName.indexOf('!')>=0) return undefined;

just after

            var resolvedModule = ts.getResolvedModule(ts.getSourceFileOfNode(location), moduleReferenceLiteral.text);

in resolveExternalModuleNameWorker function, so TS will ignore all plugin's imports.

I know it is quick-n-dirty solution, but now it allows me to perform syntax check of TS code when using SystemJS plugins.

@mark-buer
Copy link

Does the proposed solution also allow matching of modules with rooted and relative names?

I ask because all the examples given thus far only demonstrate the non-relative case.

In other words, will this:

// FILE: ambient.d.ts
declare module '*.css' {
    var r: string;
    export default r;
}

// FILE: something.ts
/// <reference path="ambient.d.ts" />
import css from './component.css';

work as expected despite the leading ./?

@mhegazy
Copy link
Contributor Author

mhegazy commented Feb 25, 2016

@mark-buer, no the current proposal does not handle that. can you elaborate on your scenario?

@olostan
Copy link

olostan commented Feb 25, 2016

@mhegazy I would expect example of @mark-buer would work, because pattern '*.css' matches './component.css'.

As use-case: I am using SystemJS with TS. And I have a lot of resources loaded with SystemJS plugin in the way: import template from './component.html!text' that is valid SystemJS import.

The relative path is required, because include is done from folder, where TS-code of component is located. It would be very strange to import css/template of component using absolute paths.

@mark-buer
Copy link

@mhegazy, @olostan summed it up nicely.

@kylecordes
Copy link

On the idea @mlynch brought up, I have often wished for the same thing but I think there are also valuable reasons for the current design. Requiring the speedbump of just a bit of extra effort to persuade typescript to let in some untyped library helps a bit to increase the community pressure on the makers of libraries to please ship typings, or if they prefer not to have typings in their pure JS project, at least put them in the @types/* mechanism. I think that pressure is a good thing, though it is hard to say what the exact optimal amount of pressure is to maximize the fraction of popular JavaScript libraries that ship with typings in the near-to-far-future.

@mlynch
Copy link

mlynch commented Oct 5, 2016

@kylecordes from an adoption standpoint, it's certainly compelling to have the force on library authors. I don't get the vibe from TS lang that they are intentionally doing that, so I feel like it's not worth the impact on developer experience.

Also, I wrote some thoughts on this issue last night and I hope we can see some movement towards a less strict, more JS ecosystem-friendly solution: https://medium.com/@maxlynch/the-one-thing-id-change-about-typescript-e76cc2eb4bd2#.o04vaqose

@testerez
Copy link

testerez commented Oct 5, 2016

IMO smoothing the new user experience and making existing projects conversion easier is a good way to bring more developer into typescript. And more typescript users means more pression on library authors to deliver definitions.

@kitsonk
Copy link
Contributor

kitsonk commented Oct 5, 2016

While it is the law of unintended consequences with this feature, it is just as "bad" as the default that noImplicitAny is off by default. In fact in this case the developer has to take active steps to "game the system" to allow them to produce poor code. You have to actively instruct TypeScript to not validate modules at design time.

@ghost
Copy link

ghost commented Oct 5, 2016

If you have the settings "moduleResolution": "node", "allowJs": true, and "maxNodeModuleJsDepth": 1, the compiler will be able to find some JS packages in your node_modules without needing a declaration. (This might not work if the package has an unusual "main" target.)

@ghost
Copy link

ghost commented Oct 5, 2016

And we have an issue open for a better solution: #11106

@mayerwin
Copy link

mayerwin commented Nov 20, 2016

I still don't understand how to properly implement a mock definition to allow the following syntax:

import { MyClass} from "my-package";

The following definition will not allow the code to compile:

declare module 'my-package' { 
    var _temp: any; 
    export = _temp; 
    export default _temp;
}

I understand the objective of forcing JS package developers to publish typings, but there should be a canonical way to make it work without it.

@mhegazy
Copy link
Contributor Author

mhegazy commented Nov 20, 2016

@mayerwin you can either define the whole module as any:

declare module "my-package";

or give it more details about the type of the export MyClass:

declare module "my-package" {
     export class MyClass {
          .....
     }
}

you can find more information about authoring declaration files at: http://www.typescriptlang.org/docs/handbook/declaration-files/introduction.html

@mayerwin
Copy link

@mhegazy Thanks 👍, I wasn't aware of the possibility to declare the whole module as any. It works. It would deserve to be highlighted more to help people know how easily they can tap into the JS ecosystem if they make the move to TS.

@wenbaofu
Copy link

wenbaofu commented Feb 1, 2017

@mhegazy : hi , I had a question about this issue
I try like this:
in file.ts I write

/// <reference path="require.d.ts" />
import JsonInfo from "json!files.json"; 

I am not sure /// need or not

then I new a file called require.d.ts

declare module 'json!*' {
    const value: any;
    export default value;
}

and the files.json is in base dir like below

index.html
files.json
app/
   -  file.ts
   -  require.d.ts

and I get below error
image
image

but I can access the json file
image

I check the https://www.bountysource.com/issues/41187251-ambient-module-declarations-with-wildcards-giving-errors-official-docs-example-not-working
and http://www.typescriptlang.org/docs/handbook/modules.html#shorthand-ambient-modules

now I have no answer,hope your reply,tks

@ghost
Copy link

ghost commented Feb 1, 2017

This feature just allows you to declare that a kind of import should work. It doesn't guarantee that it actually will work at runtime. (You're getting a 404 error, not a compile error.) That's a problem for whatever module loader you're using. (If your module loader doesn't support json! loading, it won't work.)

@wenbaofu
Copy link

wenbaofu commented Feb 1, 2017

@andy-ms : thank you first for answer

you mean systemjs (module loader) not support?

now I just use sample of hero to test json,I think loader maybe ok

this is my config

(function (global) {
    System.config({
        paths: {
            // paths serve as alias
            'npm:': 'node_modules/'
        },
        // map tells the System loader where to look for things
        map: {
            // our app is within the app folder
            app: 'app',
            // angular bundles
            '@angular/core': 'npm:@angular/core/bundles/core.umd.js',
            '@angular/common': 'npm:@angular/common/bundles/common.umd.js',
            '@angular/compiler': 'npm:@angular/compiler/bundles/compiler.umd.js',
            '@angular/platform-browser': 'npm:@angular/platform-browser/bundles/platform-browser.umd.js',
            '@angular/platform-browser-dynamic': 'npm:@angular/platform-browser-dynamic/bundles/platform-browser-dynamic.umd.js',
            '@angular/http': 'npm:@angular/http/bundles/http.umd.js',
            '@angular/router': 'npm:@angular/router/bundles/router.umd.js',
            '@angular/forms': 'npm:@angular/forms/bundles/forms.umd.js',
            // other libraries
            'rxjs':                      'npm:rxjs',
            'angular-in-memory-web-api': 'npm:angular-in-memory-web-api',
        },
        // packages tells the System loader how to load when no filename and/or no extension
        packages: {
            app: {
                main: './app.module.js',
                defaultExtension: 'js'
            },
            rxjs: {
                defaultExtension: 'js'
            },
            'angular-in-memory-web-api': {
                main: './index.js',
                defaultExtension: 'js'
            }
        }
    });
    "rxjs": "5.1.0",
    "systemjs": "0.20.5",
    "zone.js": "^0.6.25",
    "json-loader" : "^0.5.4"

pls help check if systemjs don't support or not or other problem?

@aluanhaddad
Copy link
Contributor

SystemJS uses postfix plugin syntax: e.g

import JsonInfo from "files.json!node_modules/json-loader/index.js";

There has been discussion of supporting postfix and prefix notation but I do not believe it is currently supported.

See systemjs/systemjs#1092

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Feb 1, 2017

Note that using package configuration as in

packages: {
  app: {
    main: './app.module.js',
    defaultExtension: 'js',
    meta: {
      '*.json': {
        loader: 'my-loader'
      }
    }
  },
  ....

with no plugin specifier as in

import JsonInfo from 'files.json';

is the preferred approach

@wenbaofu
Copy link

wenbaofu commented Feb 1, 2017

@aluanhaddad : if like

meta: {
      '*.json': {
        loader: 'my-loader'
      }
      import JsonInfo from 'files.json';

will give the error with cannot find the module "files.json"

image

and json-loader is same
image

@ghost
Copy link

ghost commented Feb 1, 2017

That's because SystemJS is separate from TypeScript. So you need to declare module '*.json' too.

@wenbaofu
Copy link

wenbaofu commented Feb 1, 2017

@andy-ms : yeah,it works(not give 404 error) , but when I print it , give undefined; when I give complex json, like { "name" : "test"}, then it will printf
image

image

file.ts

import JsonInfo from "files.json"; 
console.log(JsonInfo);

files.json

{
  id : 3
}

module

declare module '*.json' {
    const value: any;
    export default value;
}

systemjs

meta: {
            '*.json': {
                loader: 'my-loader'
                }
            }

@aluanhaddad
Copy link
Contributor

That is invalid JSON.

@drewlsvern
Copy link

I'm not sure if anyone solved this, but I was able to get it to work using @wenbaofu examples above. I just provided it valid json and used systemjs-plugin-json as the loader.

@Jack-Works
Copy link
Contributor

Umm, can I do something more differently?
Like

declare module "*.module.js" {
	var loader: AsyncModuleLoader
	export = loader
}

I use this type of declaration to type async module(by webpack), but not success

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests