Skip to content
This repository has been archived by the owner on Jan 5, 2024. It is now read-only.

typings search domready returns 1 result only #129

Closed
unional opened this issue Jan 27, 2016 · 21 comments
Closed

typings search domready returns 1 result only #129

unional opened this issue Jan 27, 2016 · 21 comments
Labels

Comments

@unional
Copy link
Member

unional commented Jan 27, 2016

tsd query domready -> 2 results: domready/domready and requirejs-domready/domready

typings search domready -> 1 result: domready dt https://github.com/requirejs/domReady

I'm using ded/domready so the dt file installed is not what I want (requirejs-domready module name is domReady, while ded/domready module name is domready).

How do I get ded/domeready d.ts, or how can I help to add it to the non-ambient version (better)?

I'm trying to follow the steps to create the type definition. But since there is a duplicate in this case, what should I do?

@blakeembrey
Copy link
Member

In terms of indexing, this is a bug in the API (https://github.com/typings/api). Problem is, DefinitelyTyped is a little inconsistent so I expected my parsing to fail in one or two places. Still, it's a good issue to be aware of.

To install arbitrary .d.ts files, you can install without the registry lookup. Just use typings install github:DefinitelyTyped/DefinitelyTyped/domready/domready.d.ts#3d64ea7395351f031436b52adb5cbc3906c8da4e -SA. However, it's a tiny definition and we can easily convert this.

To convert to non-ambient, just create a repo (such as typed-domready). Then make it an external module and push it to GitHub. Once on GitHub, you can add it to the registry here: https://github.com/typings/registry

declare function domready(callback: () => any): void;

export = domready;

Once it's in the registry, it'll be linked forever (or until GitHub is down) 👍

Personally, I would love if you want to get it on the registry, so if you do get stuck, let me know.

@unional
Copy link
Member Author

unional commented Jan 28, 2016

Thanks. Working on it right now.

Should I do export = domready or export default domready?

I write in typescript and currently d.ts files in tsd doesn't work with the es6 syntax import domready from 'domready' because the d.ts files do export = instead of export default (I assume you are well aware of it. :P).

Reference some of the discussions:
microsoft/TypeScript#4665
microsoft/TypeScript#4668
microsoft/TypeScript#4337

@blakeembrey
Copy link
Member

@unional Please use export =, it's semantically correct. The code is ES5 CommonJS, so export default isn't applicable.

@unional
Copy link
Member Author

unional commented Jan 28, 2016

Ok. I will do export =.
However, how to it make it work properly in typescript using vscode or atom?

Both of them say can't find module 'domready' when it is not ambient (and not doing export default).

image
image

@blakeembrey
Copy link
Member

@unional You need to use import dr = require('domread'). As for the not found, is this before you installed it using Typings? Once you install it, it should start working (beforehand it won't because, like you said, it's not ambient and doesn't understand the name yet).

@unional
Copy link
Member Author

unional commented Jan 28, 2016

I'm writing in typescript and using jspm as the module loader. require() is not a defined function in that scenario.
It is not found because vscode and atom reads the type file through tsconfig.json, and it seems to be not recognizing the non-ambient version.

tsconfig.json:

{
  "files": [
    .... 
   "typings/browser.d.ts"
  ]
}

If I change the domready.d.ts to:

...
declare module 'domready' { export = domready; }

then the module can be found but the error would become:
image

Only if I change domready.d.ts to:

...
declare module 'domready' { export default domready; }

Then it is recognized correctly:
image

@unional
Copy link
Member Author

unional commented Jan 28, 2016

Maybe I should not include typings/browser.d.ts in tsconfig.json and there is another way for vscode and atom to read the typings files?

@unional
Copy link
Member Author

unional commented Jan 28, 2016

Oh, regarding require(), I try to stay as close to ES6 as possible. :)

@blakeembrey
Copy link
Member

@unional It will not be recognised as an external module until you install it with Typings. You can install it if you need to test locally - E.g. typings install file:path/to/browser.d.ts --name domready. I'm going to be working on testing tools for type definitions over the next month - a few things for next releases still.

You should definitely include typings/browser.d.ts in files, that's the way it works.

On export = vs export default, you'll have to use export = and require(). I understand the want to stay close to ES6, but this is a module that just doesn't have a representation in ES6. If you compile the code you're using with import dr from 'domready', can you paste it here? The issue with export default and import dr is that TypeScript believes that it'll be able to correctly import the x.default property.

@unional
Copy link
Member Author

unional commented Jan 28, 2016

Ah, I understand what you mean. running typings i file:... does build the definition to:

// Compiled using typings@0.6.2
// Source: typings\domready.d.ts
declare module 'domready' {
function domready(callback: () => any) : void;

export = domready;
}

I'm been learning about typescript + jspm/systemjs + tsd for a while. It is a complicated issue and I will create a repo to showcase it. I'm using jspm with module: system.

Referencing some of the related discussions:
microsoft/TypeScript#4337
microsoft/TypeScript#4665
microsoft/TypeScript#5285
jspm/jspm-cli#1448
microsoft/TypeScript#2839
microsoft/TypeScript#4668

@unional
Copy link
Member Author

unional commented Jan 28, 2016

When I try to config the system to build with module: "commonjs", I encounter this error
image

Seems like it is related to this:
microsoft/TypeScript#5073
And in the references, there are several fix on DefinitelyType that adds namespace X to the d.ts file. e.g:
https://github.com/DefinitelyTyped/DefinitelyTyped/pull/7423/files

Should typings install ... do similar thing to the non-ambient modules?
If it should, I can create a separate issue to track that. 🌹

@unional
Copy link
Member Author

unional commented Jan 28, 2016

Here is the same issue I'm see above:
microsoft/TypeScript#3612

@blakeembrey
Copy link
Member

Thanks for all the links BTW, I'll try to come back to this once I get the chance. I do feel like this is something the compiler should fix, but I'm happy to "polyfill" various features when required before they land upstream.

@unional
Copy link
Member Author

unional commented Jan 29, 2016

Thanks!

I'm working on the sample repo for this and help me to clear my head.

IMO some collaborations are needed to "level the playground".

@blakeembrey
Copy link
Member

IMO some collaborations are needed to "level the playground".

In terms of code contribution, probably. A good place where I can add collaborators however is everything else - merging into the registry, supporting type definitions, documentation (big one).

@blakeembrey
Copy link
Member

Also, on those issues you linked, that's pretty much the gist. There's no ES6 equivalent to module.exports so semantically export = is correct. Using the namespace hack works today, but there's no future compatibility there. Can't you still use import ... = require('...') and it'll transpile correctly for however you do SystemJS? Anyway, a demo repo would be cool and get me running quicker.

@unional
Copy link
Member Author

unional commented Jan 29, 2016

import ... = require(...) won't compile under SystemJS with module: "system" (which is preferred in jspm as it is native to SystemJS thus avoid additional wrapping). In that configuration, require() is not defined.

You mentioned "they are ES5 CommonJS" above, why am I still able to write the d.ts in ES6 syntax such as:

import a from 'abc';
export * from 'abc';

Do you mean that d.ts support all syntax in TypeScript unless they can't be transpiled to ES5 CommonJS?
Is this limitation due to nodejs?

IMO the core usage of d.ts is to provide better IDE support (and maybe some tsc compile time check). In that sense, I don't see why it should be limited to ES5 CommonJS compatibility.

Or maybe introduce something that indicates who is the consumer (such as browser and using jspm or webpack), or the comsuming target?

Obviously the IDE is working fine with export default in d.ts, and I do see some d.ts are written with export default (need to find reference and put i there).

@unional
Copy link
Member Author

unional commented Jan 30, 2016

@blakeembrey , the demo is ready.
https://github.com/coolcodeexamples/timely-wealth-game
currently it is just a hello world. But it is sufficient to show case that it works.
You can run it by npm start or run it through vscode F5.
both jspm and tsc is working correctly with module: "system".

@unional
Copy link
Member Author

unional commented Feb 1, 2016

Since the original issue is fixed, I'll close this issue and open another one for the current discussion

@blakeembrey
Copy link
Member

@unional Finally got around to trying your example, it worked with TS 1.8.

@unional
Copy link
Member Author

unional commented Feb 9, 2016

Yeah, it's nice. 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants