Giter Club home page Giter Club logo

Comments (14)

unional avatar unional commented on June 26, 2024 1

What are the drawbacks of adding more files ?

It may make it harder to maintain collaboratively, and it is more work. 😄

from discussions.

unional avatar unional commented on June 26, 2024 1

Yep

from discussions.

blakeembrey avatar blakeembrey commented on June 26, 2024

@unional I don't understand the different between one file and the tsc -d statement? You're welcome to use one file if the source does not promote deep requires, but if you're using multiple files you should follow the source structure for understandability (and require-ability). If you were following the "tsc -d" statement, you'd be following the source structure anyway - because that's how tsc -d works.

from discussions.

demurgos avatar demurgos commented on June 26, 2024

Hi, I wanted to review the lodash typings today, but these are a monolithic file with almost 18k lines.
Lodash itself is in a single file (not sure, but it looks like all the commits go there and then the micro-modules get the parts they need).

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 ?

from discussions.

unional avatar unional commented on June 26, 2024

Actually it should follow however the source is written. As Blake said, for the require-ability.
So it really should be multiple files

from discussions.

demurgos avatar demurgos commented on June 26, 2024

Actually it should follow however the source is written. As Blake said, for the require-ability.
So it really should be multiple files

I stated that lodash.js is currently a single file. Did you mean "this should not be multiple files" ?

For require-ability, There should be at least one .ts for each .js if we want to allow the "package way" of requiring not only the main file but any file in the module. But what if there are more ts files ? It does not hurt the require-ability.

ie. for the following JS module foo:

./
 - package.json
 - main.js
 - bar.js

Node allows require("foo"), require("foo/main") and require("foo/bar"). If I understand, the need to copy the structure of the source is to support the latter requires. (It is not really common in node to use it for libs, but with ES6 I feel like it became convenient to use this second syntax for import {func} from "foo/bar"; instead of import * as foo from "foo"; let func = foo.bar.func;).

To fully mimic this behaviour, the typings should look like:

./
 - main.ts
 - bar.ts

But what if they look like this ?

./
 - main.ts
 - bar.ts
 - _typings-utils.ts

What are the drawbacks of adding more files ?

from discussions.

unional avatar unional commented on June 26, 2024

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?
This is also fragile as the contra-variance case could be broken. i.e.

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

from discussions.

unional avatar unional commented on June 26, 2024

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.

from discussions.

unional avatar unional commented on June 26, 2024

second syntax for import {func} from "foo/bar"; instead of import * as foo from "foo"; let func = foo.bar.func;

@demurgos , if it is ES6, probably import {bar} from "foo"; bar.func(...) would be the best, IMO.

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. 😛

from discussions.

demurgos avatar demurgos commented on June 26, 2024

This is ES6 indeed, but it's not quite the same.
I personally prefer the way it actually is with a single entry point (so you can organize your project as you want) - just like you described.
My previous comment was mainly caused by Angular 2 using deep-require during its beta phase. Since then they switched to scoped modules with a single export (npm install -S @angular/router, import {Router} from "@angular/router"). But I'm still worried that this may become a trend.

In the end, our main issue is not really how other people should export their modules but how we should type these modules.
Given the previous comments, here is your point of view as I understand it:

  1. If the package explicitly documents usages with deep-require (like Angular 2 beta), then copy the structure of the original project with definition files.
  2. Otherwise, try to write definitions for packages in a single file, even if the packages contains multiple files -> do not promote deep-require usage.
  3. If the package is too big for definitions to be written in a single file, split definitions to be a subset of the structure of the original package (so deep-require eventually works).

This seems fine to me if there is consensus.

What are the drawbacks of adding more files ?

It may make it harder to maintain collaboratively, and it is more work. 😄

I am still not sure about this. lodash is insanely big and splitting definitions to match the documentation rather than the source code would improve readability IMO. My main issue was that the definitions structure would not be a subset of the package structure, but if we decide that deep-requires should not be defined with typings (the internal structure SHOULD be private even if it CAN be accessed) then the definitions themselves could benefit from the freedom to structure them as you want.
Just to be clear, I was thinking about something like:

# Original package (lodash/lodash)
└── lodash.js

# Current definitions (typed-typings/npm-lodash)
└── index.d.ts

# Splitted definitions
├── lodash.d.ts           # Mainly a bunch of re-exports to match the API
├── ~array.d.ts
├── ~collection.d.ts
├── ~date.d.ts
├── ~function.d.ts
├── ~lang.d.ts
├── ~math.d.ts
├── ~number.d.ts
├── ~object.d.ts
├── ~seq.d.ts
├── ~string.d.ts
└── ~util.d.ts

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)

from discussions.

unional avatar unional commented on June 26, 2024

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 tsc -d. Each .ts file will produce .js and .d.ts.

If the module is "bundled" (such as lodash as you described), I would still prefer it to be separated because:

  1. Enable deep-require (which lodash does, sort of)
  2. A bundled .d.ts file created by tsc -d is not in top-level export/import declaration format, thus does not come with the benefits of the non-bundled version anyway.

🌷

from discussions.

demurgos avatar demurgos commented on June 26, 2024

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 "lodash sort of supports deep-require". Are you talking about their micro-packages ?

from discussions.

unional avatar unional commented on June 26, 2024

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.

from discussions.

unional avatar unional commented on June 26, 2024

Read more issues about this. Seems like for the export = syntax there is not much we can do. ES6 is the only way I know of:
microsoft/TypeScript#4813

Meaning we should encourage single file for commonjs syntax. PITA.

from discussions.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.