mihneadb / node-directory-tree

Convert a directory tree to a JS object.
MIT License
523 stars 106 forks source link

DirectoryTree - Missing custom field support in typescript interface #103

Closed lukasrandom closed 2 years ago

lukasrandom commented 2 years ago

Hey, just tried to add custom field to DirectoryTree object in typescript inside a DirectoryTreeCallback. The following error occured:

Error:

error TS7053: Element implicitly has an 'any' type because expression of type '"id"' can't be used to index type 'DirectoryTree'.
  Property 'id' does not exist on type 'DirectoryTree'.

34     item['id'] = createHash('sha1').update(path).digest('base64');

Example adding custom id field:

import directoryTree, {
    DirectoryTree,
    DirectoryTreeCallback,
} from 'directory-tree';
import { createHash } from 'crypto';

const path = 'C:/Users/me/Desktop/a';
const callback: DirectoryTreeCallback = (item: DirectoryTree, path: string) => {
    item['id'] = createHash('sha1').update(path).digest('base64');
};
const dirTree: DirectoryTree = directoryTree(path, {}, callback, callback);
console.log(JSON.stringify(dirTree, null, 2));

Fix:

Added [key: string]: any in to the DirectoryTree interface at node_modules\directory-tree\index.d.ts. I will send you a PR so you can decide if this is a reasonable fix for it.

Best Lukas

lbragile commented 2 years ago

@lukasrandom Seems like this was merged with the fix you indicated, but even though it solves the issue you faced, I feel like that doesn't really provide meaningful typing to the custom object.

That is, when I add properties to the custom object, they now all have type any, which doesn't help with intellisense.

Would it not be better - developer experience wise - to make the DirectoryTree interface (and thus DirectoryTreeCallback type) generic as shown below?

// node_modules/directory_tree/index.d.ts

import { Stats } from 'fs';

interface IObj { 
  [key: string]: any;
}

declare function directoryTree<TCustomFile extends IObj = IObj, TCustomDir extends IObj = IObj, TCustomResult = TCustomFile & TCustomDir>(
  path: string,
  options?: directoryTree.DirectoryTreeOptions,
  onEachFile?: directoryTree.DirectoryTreeCallback<TCustomFile>,
  onEachDirectory?: directoryTree.DirectoryTreeCallback<TCustomDir>,
): directoryTree.DirectoryTree<TCustomResult>;

export as namespace directoryTree;

declare namespace directoryTree {
  export interface DirectoryTree<C extends IObj = IObj> {
    path: string;
    name: string;
    size: number;
    type: "directory" | "file";
    children?: DirectoryTree<C>[];
    extension?: string;
    isSymbolicLink?: boolean;
    custom: C;
  }

  export interface DirectoryTreeOptions {
    normalizePath?: boolean;
    exclude?: RegExp | RegExp[];
    attributes?: (keyof Stats | "type" | "extension")[];
    extensions?: RegExp;
    followSymlinks?: boolean;
    depth?: number;
  }

  export type DirectoryTreeCallback<TCustom extends IObj = IObj> = (item: DirectoryTree<TCustom>, path: string, stats: Stats) => void;
}

export = directoryTree;

This should not change the current functionality, but will allow developers to have more fine grained control over the item.custom object.

For example, if I don't want the custom object to have all it's properties typed as any, I can simply declare my own type:

export type TDirectoryTree = DirectoryTree<TCustomFile & TCustomDir>;

and use it throughout my app instead of the default DirectoryTree type

@mihneadb if you agree with the above I can make a PR 🙂

mihneadb commented 2 years ago

@lbragile sorry about the terribly late reply - I think this would complicate use more for the "average user". Thoughts of other options perhaps?

lbragile commented 2 years ago

@mihneadb No worries. I will have to disagree with you on:

I think this would complicate use more for the "average user"

as my proposed typing should not cause anyone to update anything due to the same default types. That is, what I suggested only provides users the ability to "opt into" custom typing if they choose to - hence the generic interfaces.

On that note, I decided to drop this package in favor of a custom solution for my needs, so feel free to use the above if you'd like. If you do end up using it, I would appreciate a note crediting me 🙂 Or I can create a PR whichever works better for you.

mihneadb commented 2 years ago

@lbragile my bad, I misread your proposal as something that would implicitly require folks to provide explicit typing always. Thanks for having the patience to reply to that. :) A PR sounds great!

lbragile commented 2 years ago

@mihneadb Sounds good, created a PR. Please have a look and let me know if there is anything that you would like to adjust