KartikTalwar / gmail.js

Gmail JavaScript API
MIT License
3.74k stars 455 forks source link

Using types from gmail.d.ts #678

Closed DrParanoia closed 2 years ago

DrParanoia commented 2 years ago

Sorry if this is a stupid question, but I kinda hit a dead end.

Let's say I have this code this.gmailJs.observe.on("view_email", (domEmail) => {...

How can I specify that domEmail is of type GmailDomEmail? It seems that I can't get to that type at all :( Thanks!

josteink commented 2 years ago

If your Typescript compiler is setup correctly, all you should need to do is add the type-annotation for that parameter:

this.gmailJs.observe.on("view_email", (domEmail: GmailDomEmail) => {...

Or are you not getting types loaded at all?

DrParanoia commented 2 years ago

@josteink Thanks for the answer! Well, to add that type annotation I need to import the type, but since there are no exports in gmail.d.ts file, it's not considered a module.

Right now, as a solution, I just copied the gmail.d.ts file and added export to the declare class Gmail line at the bottom of the file. It then allows me to do this import type {Gmail, GmailDomEmail, GmailDomCompose} from './@types/gmail'.

Maybe export should be added to the Gmail class in your repo also which would allow importing types from gmail-js? Or there are side effects I don't know about?

josteink commented 2 years ago

I'm literally not doing anything in particular to get those types available in my IDE of choice (Emacs) and to get the compiler to recognize or enforce them.

I don't have to import the types at all. This is the code I have in my GmailJS pre-loader:

// gmail.js needs to be loaded as early as possible to be able to intercept
// embedded email-data in the gmail HTML!
//
// To do that we:
// - just load it
// - make sure it's initialized
// - and do nothing else!
//
// Let the "big" GmailLink extension bundle load separately!

// eslint-disable-next-line @typescript-eslint/no-var-requires
const GmailFactory = require("gmail-js");
// eslint-disable-next-line @typescript-eslint/no-var-requires
const jquery = require("jquery");

const ns = (window as any).ProductName || {};
ns.Vendor = {};
ns.Vendor.Gmail = new GmailFactory.Gmail(jquery) as Gmail;
(window as any).ProductName = ns;

And in my extension I can just cast that to value Gmail later on and get type-safety for everything else automatically:

export class GmailHandler {
    gmail: Gmail;

    // ... snip

    constructor() {
        this.gmail = (window as any).ProductName.Vendor.Gmail as Gmail;

I literally didn't have to configure anything for it, at least not as far as I can remember.

Can you share the repo you are using where are struggling to get this working?

Or at least a minimal POC demonstrating your issues? :)

DrParanoia commented 2 years ago

@josteink I've created a stripped down repo at https://github.com/DrParanoia/chrome-gmail-demo.

Basically, I am using PHPStorm. And for example, you can look at src/interface/demo-interface.ts where I used my copied gmail.d.ts file "hack".

Without the "hack", how can I specify types for the interface properties? Also, without the "hack" autocomplete does not work in PHPStorm.

Or, for example, if you look at the src/component/page-change-watcher.ts, how can I specify, that the constructor parameter is of type Gmail without creating a new instance and casting it as Gmail without the "hack"?

I appreciate your help by the way, thanks! πŸ˜„

josteink commented 2 years ago

It seems like I added a file called types.d.ts to my project which I forgot about. It basically consists of this one-liner:

import "gmail-js";

That does the trick, and works in your repo too! (I've tested :smile: )

DrParanoia commented 2 years ago

@josteink Thanks for the advice, will try it out! πŸ˜† On a side note, maybe it's worth it to include it by default in the package (or maybe just export the Gmail class in the gmail.d.ts file)? Would be nice if types would work out of the box πŸ˜†

DrParanoia commented 2 years ago

If you simply add export to declare class Gmail in the gmail.d.ts file, it also allows you to use the library like this


import $ from "jquery";
import {Gmail} from 'gmail-js';

let gmailJs = new Gmail($);
josteink commented 2 years ago

Unfortunately Gmail.js itself does not use β€œreal” classes and β€œreal” exports, so that mighty create a mismatch between the β€œreal” code and the type-definitions.

Before implementing such a change, I’d have to look over it quite thoroughly and make sure:

To be clear, I’m not against such a change, but I’m not going to do it on a whim either.

PRs welcome, I guess πŸ˜…

DrParanoia commented 2 years ago

Sure πŸ˜† Well, at least the types.d.ts file workaround works πŸ˜… Maybe at least add it to the readme? 🀣

josteink commented 2 years ago

Just did 1 hour ago πŸ˜‰

DrParanoia commented 2 years ago

@josteink Nice! Thank you very much! πŸ˜†

josteink commented 2 years ago

Great. Considering this issue closed then πŸ™‚

josteink commented 2 years ago

So I got curious and did some testing here.

I've started the work on a v2.0 with breaking changes, and it lives in the vnext-branch.

I've tested that with my code. With that I could remove my types.d.ts hack, and use it like a "normal" NPM-package, and I think that is a great improvement, although an incompatible one! :smile:

Feel free to try it out and let me know what you think!

DrParanoia commented 2 years ago

@josteink Looks nice! I mean the types.d.ts hack is pretty nice (don't have to import any types to use them) πŸ˜† But I feel like importing the types is the proper way of doing it 🀣

Will there be a lot of breaking changes for v2.0? I am currently working on a project for a Gmail Chrome extension, and I think I'll stick with the "hack" for now, simply so that nothing would break in the middle of development πŸ˜„

josteink commented 2 years ago

"Planned" breaking changes so far mostly involve removing known broken or useless functions, and some non-async method-forms people shouldn't be using anyway.

In short, if you are:

Then the risk of using the newer code should be absolutely minimal. But then again, the risk in a future upgrade should be similarly small.

No shame in using the current production version for now, if you want to play it safe.