Skip to content

Distribute power-assert.d.ts(index.d.ts) type definition files using npm #29

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

Closed
wants to merge 4 commits into from

Conversation

falsandtru
Copy link
Contributor

Enhanced node package distribute system. Distribute package with .d.ts type definition files using npm.
https://github.com/Microsoft/TypeScript/wiki/Typings-for-npm-packages

Type definition files might well survive even after typescript died.
http://mozaic.fm/post/96461640633/8-altjs

@falsandtru falsandtru changed the title Distribute type definition files using npm Distribute power-assert.d.ts(index.d.ts) type definition files using npm Oct 4, 2015
@twada
Copy link
Member

twada commented Oct 5, 2015

@falsandtru Thank you for your pull-req!

Since this is a relatively bigger patch, and I don't know much about TypeScript, I would like @vvakame and @teppeis to review this pull-req.

Would you review this PR? > @vvakame @teppeis

@vvakame
Copy link
Member

vvakame commented Oct 5, 2015

I think @twada should be reject this pull request if @twada can't review about .d.ts.
DefinitelyTyped give maintainance for all .d.ts. DefinitelyTyped is useful for npm package owner when he is not TS user.

@falsandtru
Copy link
Contributor Author

DefinitelyTyped is too large project because many pull requests are left unused. .d.ts files should divide and rule the management and maintainance model. So Microsoft created the new model. We should shift to the this new model and self-controllable smart eco system. I want this model if @twada can. By the way, I thought of a good idea. Please wait a few days. I show interesting things.

@teppeis
Copy link

teppeis commented Oct 5, 2015

It's trade-off. Hosting index.d.ts ourself is nice for TypeScript users because they don't have to take care of DefinitelyTyped repo additionally.

However, this PR includes tripleslash that is NOT allowed in index.d.ts.

error TS2654: Exported external package typings file cannot contain tripleslash references. Please contact the package author to update the package definition.

In Typings for npm packages · Microsoft/TypeScript Wiki

Typings file should:

  • be a .d.ts file
  • be an external module
  • not have triple-slash references

Before power-assert itself, the dependencies (empower and power-assert-formatter) should host their own index.d.ts. Then power-assert can import them.

@falsandtru
Copy link
Contributor Author

Yes. My triple-slash is temporary measures because we cannot resolve type definition dependencies only in this repository. I will resolve this problem in the future. This PR's target is to resolve the hosting(distribute) and maintainance(care) problem. I'll resolve all .d.ts dependencies quickly if you want.

@falsandtru
Copy link
Contributor Author

Fixed email of commit metadata. And I cancel my idea because need an other server for security.

@twada
Copy link
Member

twada commented Oct 8, 2015

Hi @falsandtru , @vvakame and @teppeis ,
Thank you so much for your discussion, and sorry for my late response.

And I cancel my idea because...

Okay, I'm going to close this pull-req for now.

However, your suggestion make me think about TypeScript users a lot.
I hope power-assert to be convenient to both JavaScript users and TypeScript users.
Hosting .d.ts may be a solution, so someday we host type definitions by ourselves.

Thank you for your pull request!
Please feel free to send us more requests.

@twada twada closed this Oct 8, 2015
@falsandtru
Copy link
Contributor Author

Oh it is not mean this pull request. Please continue.

@twada
Copy link
Member

twada commented Oct 18, 2015

@falsandtru I'm so sorry for my very late response.

Today I have a conversation with some TS users, and now I'm going to accept your suggestion.
However, I cannot accept your pull-req As-Is, since this PR should solve some more problems that @teppeis has pointed out.

So, please wait a few days.
I'll show you an update direction of this pull-req.

@falsandtru
Copy link
Contributor Author

Okay, I thought that this is my strongest pull-req(without behind-the-scenes maneuvering).

@falsandtru
Copy link
Contributor Author

Fixed @teppeis specified problems. This commit need typings of depended npm packages. Please reopen to show my fixed commit.

@twada
Copy link
Member

twada commented Oct 20, 2015

@falsandtru Sorry for my late response.

Fixed @teppeis specified problems. This commit need typings of depended npm packages. Please reopen to show my fixed commit.

Okay, I'll reopen this PR. Looking forward to see your update.

@vvakame
Copy link
Member

vvakame commented Oct 21, 2015

I want to confirm what is the goal of this pull request?
I think We want to use power-assert simply in commonjs module context.. isn't it?
My recommend is along the TypeScript team intention.

https://github.com/Microsoft/TypeScript/wiki/Typings-for-npm-packages#typings-file-should

@falsandtru
Copy link
Contributor Author

My goal: https://github.com/Microsoft/TypeScript/wiki/Typings-for-npm-packages

If this pull request has a technical problem, I provide some ways for resolution. Please tell me your wants.

@vvakame
Copy link
Member

vvakame commented Oct 21, 2015

sounds good. We are sharing a the goal. 😸
My opinition is same as teppeis one. #29 (comment)

Before power-assert itself, the dependencies (empower and power-assert-formatter) should host their own index.d.ts. Then power-assert can import them.

We should distributed index.d.ts to empower, power-assert-formatter and power-assert.
index.d.ts should external module. not ambient external module.

https://github.com/Microsoft/TypeScript/wiki/Typings-for-npm-packages#typings-file-should

be an external module

https://github.com/Microsoft/TypeScript/blob/master/doc/spec.md#12.2
I think ambient module declaration is not strict equals to external module.

@falsandtru
Copy link
Contributor Author

My opinition is same as teppeis one. #29 (comment)

I resolved that problem. This resolution refer to external modules directly. Please review again.
https://github.com/power-assert-js/power-assert/pull/29/files.

index.d.ts should external module. not ambient external module.

I'll fix module design if @twada want.

Sample:

// index.d.ts
declare function assert(value:any, message?:string): void;
declare namespace assert {
  ...
}
export default assert;

Not ambient external module is minimal, but not good design for model case. Maybe many non-typescript node module developers cannot/doesn't design own .d.ts file.

DefinitelyTyped:

  • can update from DefinitelyTyped
  • hosted by DefinitelyTyped
// foo.d.ts
declare namespace foo {
  export var fizz;
}
declare module "foo" {
  export default foo;
  export var fizz: typeof foo.fizz;
}

External modules (compatible):

  • can update from DefinitelyTyped
  • hosted by owner or DefinitelyTyped
// foo.d.ts
declare namespace foo {
  export var fizz;
}
declare module "foo" {
  export default foo;
  export var fizz: typeof foo.fizz;
}
// index.d.ts
import "./foo.d.ts";
export * from "foo";
export {default} from "foo";

External modules (for @vvakame):

  • cannot update from DefinitelyTyped
  • hosted by owner
// index.d.ts
declare namespace foo {
  export var fizz;
}
export default foo;
export var fizz: typeof foo.fizz;

I recommend compatible design.

@falsandtru
Copy link
Contributor Author

And @vvakame please read this point: http://qiita.com/vvakame/items/72d22e33632178f7db24#comment-482f292bbd35e004c74e

いくつか調べたところ、外部モジュールではexportもimportもない型定義ファイルが外部モジュールではないと判定されてエラーとなる制約と、export/importとdeclare module "hoge"を同じファイルに記述することができないという制約が個別にあり、この2つの制約が同時に適用される条件下にある結果として外部モジュールとして直接参照されるindex.d.tsファイルでdeclare module "hoge"が使用できなくなっています。

つまり、外部モジュールの定義においてdeclare module "hoge"の使用は禁止されておらず、間接的であればdeclare module "hoge"を使用できる現在の動作は制約漏れでなく正しいものであり、トリプルスラッシュと同じ根拠で将来において禁止される可能性のあるものではないことを書き残しておきます。

@twada
Copy link
Member

twada commented Oct 22, 2015

@falsandtru @vvakame @teppeis

Thank you for your discussions. Thank you all, really.

Decided. I close this pull-req for now.

I think this PR is not bad. Hosting index.d.ts will be one of the future direction we select.

However I close this PR because,

  1. TypeScript spec and implementation of hosting typings in npm module seems to be young, not stable, not mature enough. There should be only one true way to describe and provide type definitions.
  2. I (@twada) am not a TypeScript user, so I cannot review deep technical details of this PR for now. It's my fault.
  3. There are some other gradual typing solutions like Flowtype. TypeScript is not a dominant language yet.

So, I want to wait for time-proven and well-documented best practices of hosting type definitions. I'll reopen this or send another pull-req when the time for hosting typings has come.

I'm so so sorry for closing this pull-request.

Thanks.

@twada twada closed this Oct 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants