gildas-lormeau / zip.js

JavaScript library to zip and unzip files supporting multi-core compression, compression streams, zip64, split files and encryption.
https://gildas-lormeau.github.io/zip.js
BSD 3-Clause "New" or "Revised" License
3.39k stars 510 forks source link

Wrong types for FS #324

Closed sebastjon closed 2 years ago

sebastjon commented 2 years ago

I wonder if the types for FS are correct. In your code FS and all types of ZipEntry are classes. However, only the interfaces are exported for Typescript.

I would like to call const fs = new zip.fs.FS().

This is a problem because zip.fs.FS looks like an instance of the class according to the types, but it is actually the class itself.

I would therefore create a pull request that exports the classes (with constructors) instead of the interfaces.

This would also mean that this line changes from: export const fs: { FS: FS, ZipDirectoryEntry: ZipDirectoryEntry, ZipFileEntry: ZipFileEntry };

to: export const fs: { FS: typeof FS, ZipDirectoryEntry: typeof ZipDirectoryEntry, ZipFileEntry: typeof ZipFileEntry };

Is this right or am I thinking wrong?

gildas-lormeau commented 2 years ago

I think you're right, the autocomplete in VSCode agrees with you too. Actually, I don't write TypeScript code and I did not know that classes were instances of interfaces. I don't understand why typeof is needed, I guess I need to read the manual.

If you would like to create a PR, I will be happy to merge it, otherwise I can apply the fix for you.

mon commented 2 years ago

I also had trouble (even after a manual cast to the FS type) using .entries and using some file attributes like .name, so I gave up and migrated my code to the ZipReader class 😅 The PR would be appreciated!

sebastjon commented 2 years ago

I created a PR.

I don't understand why typeof is needed, I guess I need to read the manual.

typeof operator

TypeScript adds a typeof operator you can use in a type context to refer to the type of a variable or property

Does this information help?

gildas-lormeau commented 2 years ago

No, the documentation does not really help me. Actually, what is the result of typeof FS? The interface of the class? In JS, the type of a class is function.

PS: I just (re?)discovered the type keyword. I guess I need to start here...

sebastjon commented 2 years ago

I'll try to explain by this example. You can play around with it in the TS playground.

Let's say we have a simple User class:

class User {

    name: string;

    constructor(name: string) {
        this.name = name;
    }

    greet() {
        return `Hello, I am ${this.name}`;
    }
}

Now, let's play arround with a few assignments:

const userInstance = new User('alice');

// JS semantic
console.log('userInstance is an', typeof userInstance); // 'object'

const userInstanceWithExplicitTypeAnnotation: User = new User('bob');
// and still in JS semantic:
console.log('userInstanceWithExplicitTypeAnnotation is an', typeof userInstanceWithExplicitTypeAnnotation); // 'object'

const UserClass = User;
// JS semantic
console.log('UserClass is a', typeof UserClass) // 'function'

// In the next line, `typeof` is used in a different semantic - which only provides Typescript for its type system:
const UserClassWithExplicitTypeAnnotation: typeof User = User;
// and still, in JS semantic:
console.log('UserClassWithExplicitTypeAnnotation is an', typeof UserClassWithExplicitTypeAnnotation); // 'function'

// wrong type:
const UserClassWithWrongTypeAnnotation: User = User;
// error: Property 'greet' is missing in type 'typeof User' but required in type 'User'

const objectWithTypeAnnotation: {
    instance: User,
    clazz: typeof User,  // this is just the type annotation in TS semantic
} = {
    instance: new User('charly'),
    clazz: User, // this is the real assignment
}

Now let's have a look at this project:

/*
In index.d.ts we have something like:
*/

class ZipFileEntry {/*...*/}

class ZipDirectoryEntry{/*...*/}

class FS {/*...*/}

// In index.d.ts we only use type annotations. The assignment is only made in zip-fs-core.js.
export const fs: { FS: typeof FS, ZipDirectoryEntry: typeof ZipDirectoryEntry, ZipFileEntry: typeof ZipFileEntry } = {
    FS: FS,
    ZipDirectoryEntry: ZipDirectoryEntry,
    ZipFileEntry: ZipFileEntry
};
gildas-lormeau commented 2 years ago

Thank you very much! I played around with the TS playground and your examples a bit, and I think I get it out now. The FS property stores a class so its type is the type of the class, and this latter type is Function as illustrated with the code below:

const UserClassWithoutTypeof: Function = User;

I also discovered here that I could extend Function to benefit from type inference.

interface Type<T> extends Function { new (...args: any[]): T; };

const UserClassAngularLike: Type<User> = User;