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

Prefer structure to expose types #354

Closed
unional opened this issue Mar 20, 2016 · 13 comments
Closed

Prefer structure to expose types #354

unional opened this issue Mar 20, 2016 · 13 comments

Comments

@unional
Copy link
Member

unional commented Mar 20, 2016

From handbook: https://github.com/Microsoft/TypeScript-Handbook/blob/master/pages/Writing%20Definition%20Files.md#namespacing

It's a judgement call to place interfaces/types inside a namespace.
For usability, I often find exposing types at top-level distracting. For example, when using lodash:

import _ = require('lodash');

_ completion will include interfaces:
image

Would it be a good idea to suggest we all put the types inside a namespace such as Typings (inside the module, not a global namespace)?

This only applies to types/interfaces that are not already exposed directly.

@blakeembrey
Copy link
Member

Maybe. I guess I'd have to mull over it for a bit and get others feedback. I'm not directly opposed, but the discoverability is nice for me with top-level interfaces.

@unional
Copy link
Member Author

unional commented Mar 28, 2016

One drawback is if it is a util package, and there is name collision between the type and variable. In that case what is the recommend approach?

class A() {...}

export = {
  A: new A();  // best if the property is `a`, but package author decided to expose as `A`
}

@blakeembrey
Copy link
Member

@unional You're talking about if the obvious type name is already a value? I assume then that they don't export the original type so maybe it's ok. In the case where we're legitimately stretched, I prefix is common.

@unional
Copy link
Member Author

unional commented Mar 29, 2016

I prefix is common, but: https://github.com/Microsoft/TypeScript-Handbook/blob/4439d3101283adb38dabb2a4c39726986d6bbcb2/pages/Writing%20Definition%20Files.md#naming-conventions

Don't want to foster un-recommended pattern. Maybe suffix it with Interface | Class?

@johnnyreilly
Copy link
Member

This would be a problem with Angular 1.x codebases. In that case most services/providers are exposed as interfaces under ng. You want them there as when creating Angular 1.x style modules you use those interfaces to provide the type annotations to the injected members. So at least in that case you'd certainly want interfaces exposed.

@johnnyreilly
Copy link
Member

This may already be covered by your caveat btw

@unional
Copy link
Member Author

unional commented Mar 29, 2016

Yeah. The "best" way I can think of is have a conventioned namespace in the module, e.g. Interfaces or Typings:

class Foo {}
declare namespace Foo {
  namespace Typings {
    export interface Boo {}
  }
}

export = Foo;

@unional
Copy link
Member Author

unional commented Apr 3, 2016

Counter argument:
namespace Interfaces/Typings is purely artificial and it does not reflect how things are exposed IF the package is written in TypeScript and distributed with .js and .d.ts.

Therefore the types should be exported along the code, i.e. the original way. When there is conflict, go the suffix route.

@blakeembrey
Copy link
Member

Another thing against making the abstract namespace - TypeScript already bundles interfaces alongside regular functions. E.g. RegExpExecArray.

@unional
Copy link
Member Author

unional commented Apr 4, 2016

Yeah, that pretty much settles it. 👍

@asvetliakov
Copy link

Using currently constructions like these:

import * as sinon from "sinon";
let myStub: sinon.SinonStub; // this is pure interface
beforeEach(() => {
   myStub = sinon.stub() //sinon.createStubInstance();
});
// do things

i don't want to write

let myStub: sinon.Interfaces.Typings.SinonStub // or
let myStub: Interfaces.Typings.SinonStub

Another thing, some lib may define object interface, like MyLibConfiguration. Sometimes it's useful to type stuff with this interface not in context where this function is being invoked:

let myObj: mylib.MyLibConfiguration = { ... }
myLib(myObj);

and again, don't want to do this:

let myObj: Interfaces.Typings.MyLib.MyLibConfiguration = { ... }

@unional
Copy link
Member Author

unional commented Apr 11, 2016

Yup. It is pretty much settled. While it is not as bad as you describe (it's mylib.Interfaces.MyLibConfiguration or sinon.Typings.SinonStub), it is better to think in:

"how would it look like if the package is actually written in TypeScript and compiled using tsc -d"?

@blakeembrey
Copy link
Member

Awesome, feel free to close @unional.

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

No branches or pull requests

4 participants