TypeScript rewrite

[atomiks]

Resolves #378

cc @KubaJastrz, also how do I use a single declaration file (./types.ts and index.d.ts are duplicates?)

[KubaJastrz]

Definitely a good start. There are still many as and any keywords that could be replaced. And .eslintrc is not set up correctly for typescript plugin.

I'm gonna do a full review later today locally and check what can be written more strictly.

[atomiks]

One thing that really confuses me is the angle bracket syntax <>. I don't understand it at all. I read the docs for it and it's meant to be like as(?) but I don't get it, yet see it everywhere 🤯

[KubaJastrz]

They can be used for type casting, but only in .ts files (that is not the case for .tsx in React), so I recommend not using it this way at all (just use as keyword instead).

Another use that you might see (basically all the time) is as generic type. It works sort of like a function parameter. More info here.

[atomiks]

Yeah I thought the generic type would be good for the util functions but seemed to cause problems so just fell back to any :. Anyway look forward to your improvements 🙂

[KubaJastrz]

That was quite a ride. Probably missed a few things 😅
Good job 👍

[atomiks]

Thank you tons for the help @KubaJastrz ! I think we've caught a few obscure bugs thanks to TS 😎

It seems like there are lots of these errors now:

[ts] Cannot invoke an object which is possibly 'undefined'. [2722]

[ts] Object is possibly 'null'. [2531]

[ts] Object is possibly 'undefined'

With regards to the instance props, etc. Any ideas of what to do?

[atomiks]

I think we need to distinguish the Props interface and Options

Like this?

export interface Props {
  [all required, no ?:]
}

export type Options = Partial<Props>

That would probably be a breaking change for the types though...? :/

[atomiks]

Managed to get all types passing 🤷‍♂️

Had to hack the EventListener type for the on* trigger listeners. I'm sure there's a better way

[KubaJastrz]

There are still some Function types to remove.

We could drop index.d.ts and replace it with --declaration flag during the build process. I'll look into it.

[atomiks]

We could drop index.d.ts and replace it with --declaration flag during the build process. I'll look into it.

We need to keep the Props backwards-compat though. So I think we should keep it separate for this major?

[KubaJastrz]

There's something weird going on. We have our internal types in src/types.ts. That's where I proposed to create Defaults interface that just mirrors the object in src/defaults.ts.

So the flow would be something like this:

// in src/types.ts
import defaults from './defaults'
type Defaults = typeof defaults
interface Props extends Defaults /* maybe omit some props */ {
  /* and override them here, like we do in @tippy.js/react */
}

No need for Options interface.

Because we're using typescript, we could potentially drop index.d.ts and have it be generated automatically during build process (I hope this can be done without breaking changes, not sure).

[atomiks]

Well ideally we don't have a Defaults interface. I only added it as part of index.d.ts backwards compat. We should rename that file I guess and move props.ts stuff into utils.ts.

There should be two interfaces: Props and Options = Partial<Props>

[atomiks]

@KubaJastrz I think this is pretty decent now? I'll keep index.d.ts and ./types.ts separate for now, we can work out what to do in next major. Currently they're synced with some internal/private types taken away.

[KubaJastrz]

Well ideally we don't have a Defaults interface.

So why do we have it now? I thought that could be used for fixing this, but currently it's just a copy of previous Props interface (without ?).

[atomiks]

So why do we have it now? I thought that could be used for fixing this, but currently it's just a copy of previous Props interface (without ?).

Cuz I copy-pasted the Props interface from ./types which included a couple fixes 😛 Will be easier to keep them in sync if I don't need to convert them to be optional in index.d.ts

Moving forward, people should import Options instead of Props to represent the optional interface which will work with this major. I'll rename the merged / full props interface to WholeProps for now in index.d.ts? People seemed to work fine without a full Props interface anyway before.

[atomiks]

Going to merge this in now as linting, types and tests pass fine, I'll do a minor release in the next week sometime. Feel free to make some PRs in the meantime to improve the current types 🙂

[KubaJastrz]

I don't know what WholeProps would be useful for. I think it's unnecessary because you can easily use something like this in the userland :

type WholeProps = Required<Options>

I suggest just sticking to Props -> Options rename if you really want it.

[atomiks]

It types instance.props (default props + user-supplied options), but other interface use Options (only user-supplied options).

People were doing #408;

import { Props } from 'tippy.js'

But it will break if we change Props to be Required<Options> (or use non-optional interface)?