-
Notifications
You must be signed in to change notification settings - Fork 0
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
typings file structure best practices #15
Comments
@unional I don't understand the different between one file and the |
Hi, I wanted to review the lodash typings today, but these are a monolithic file with almost 18k lines. I see here that you want to promote single-file typings or replication of the original structure, but what if the typings declarations were split accross a few files (ie. their documentation define a few groups of functions - Array, Collection, Object, etc.) and then merged and exported from a single file ? Something along the lines of: // lib/array.ts
export interface __LoDashStaticArray {
chunk<T>(array: List<T>, size?: number): T[][];
concat<T>(...values: (T[] | List<T>)[]): T[];
// ...
}
// lib/string.ts
export interface __LoDashStaticString {
camelCase(string?: string): string;
capitalize(): string;
// ...
}
// lodash.ts
import {__LoDashStaticArray } from "./lib/array.ts";
import {__LoDashStaticString} from "./lib/string.ts";
declare var _: _.LoDashStatic;
export namespace _ {
export interface LoDashStatic extends __LoDashStaticArray, __LoDashStaticString {}
LoDashStatic {
(value: number): LoDashImplicitWrapper<number>;
(value: string): LoDashImplicitStringWrapper;
// ...
}
}
export = _; In short, this is not about extending the global namespace or adding a publicly accessible sub-namespaces (as discussed in the linked issues), but rather using multiple private files to organize the definitions. Can I use this sort of structure/pattern or do you prefer to stick with big files ? |
Actually it should follow however the source is written. As Blake said, for the require-ability. |
I stated that For require-ability, There should be at least one ie. for the following JS module
Node allows To fully mimic this behaviour, the typings should look like:
But what if they look like this ?
What are the drawbacks of adding more files ? |
It may make it harder to maintain collaboratively, and it is more work. 😄 |
When I follow the source code and break code into their own file, I am having difficulty in re-exporting them: import Emitter = require('./emitter');
import Disposable = require('./disposable');
declare namespace EventKit {
// export Emitter <-- how to export?
// export Disposable
}
export = EventKit; I don't want to rely on ES6/commonjs interop: export * from './emitter';
export * from './disposable'; The following partially works. I want to avoid the extra level of extends: import EmitterS = require('./emitter');
import DisposableS = require('./disposable');
import CompositeDisposableS = require('./composite-disposable');
declare namespace EventKit {
export class Emitter extends EmitterS { }
export class Disposable extends DisposableS { }
export class CompositeDisposable extends CompositeDisposableS { }
...
}
export = EventKit; Any suggestion? declare namespace EventKit {
...
}
declare class EventKit {
static foo(): DisposableS
}
// usage
import EventKit = require('event-kit');
// error: DisposableS is not assignable to EventKit.Disposable
let x: EventKit.Disposable = EventKit.foo(); https://github.com/typed-typings/npm-event-kit/blob/master/lib/event-kit.d.ts#L7-L9 |
A "workaround" for the contra-variance case would be this: declare namespace EventKit {
export class Disposable extends DisposableS {}
...
}
declare class EventKit {
static foo(): EventKit.Disposable
} But the deep-require case would still break. |
@demurgos , if it is ES6, probably IMO deep-require is mainly for commonjs/nodejs module usage. ES6 module should probably export what they want to export without relying user to understand their inner file structure. 😛 |
This is ES6 indeed, but it's not quite the same. In the end, our main issue is not really how other people should export their modules but how we should type these modules.
This seems fine to me if there is consensus.
I am still not sure about this.
The tilde is just a proposition to denote that it is an internal file used for the definitions that do not correspond to a file in the original package. (It would be coherent with the internal module ids generated during the installation of definitions) |
My point of view (currently and likely final, unless #15 (comment) can't be resolved easily) is point 1. Just mimic the original structure. That's how it will be ended up with if the module was written in TypeScirpt and compiled using If the module is "bundled" (such as
🌷 |
Ok, so even if the documentation does not provide any guarantees about deep-require, the definitions' structure should always be a superset of the original project ? (Ideally it would match the original structure and eventually it would have some extra internal files) Could you expand on what you mean by " |
Yep |
One benefit for closely matching the file structure of the source is that it can be transferred to the source easily. Of course, how practical is it to do so for the typings is another thing to consider. |
Read more issues about this. Seems like for the Meaning we should encourage single file for commonjs syntax. PITA. |
Originally I tend to organize typings in multiple files so that they are more manageable. e.g.
npm-chai
.But now through the conversation in UMD, and best practice of exposing types,
it seems like it is actually best to keep them in 1 file (or 2, if we can't nicely solve the UMD issue microsoft/TypeScript#7125 and microsoft/TypeScript#7156)The key statement is this:
"how would it look like if the package is actually written in TypeScript and compiled using
tsc -d
"?It has the following benefits:
lib
orsrc
? Nested or mirror source structure?).d.ts
file can easily be moved into the source package when package author agreesRelevant discussions:
typings/typings#402
typings/typings#10
typings/typings#354
The text was updated successfully, but these errors were encountered: