Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TS: Bundle Type definition for use-analytics #51

Open
thilak-rao opened this issue May 14, 2020 · 11 comments · May be fixed by #389
Open

TS: Bundle Type definition for use-analytics #51

thilak-rao opened this issue May 14, 2020 · 11 comments · May be fixed by #389

Comments

@thilak-rao
Copy link
Contributor

It would be great if use-analytics npm package can bundle type definitions. It would help me get rid of this from my code:

https://gitlab.com/trao/trao.dev/-/blob/master/global.d.ts#L11

I would be happy to file a PR to fix this.

@DavidWells
Copy link
Owner

DavidWells commented May 17, 2020

Sounds good to me. Happy to have a PR on this 😃

How would this work exactly? Adding a use-analytics.d.ts file, like this, to the use-analytics package?

Thanks!

@thilak-rao
Copy link
Contributor Author

I see one of two ways to do this:

  • Migrated use-analytics leaf package in this repo to TypeScript
  • Add ambient type declaration file into use-analytics like how analytics-core does it

How do you think I should go about implementing it?

@thilak-rao
Copy link
Contributor Author

I have never used tsd-jsdoc to generate ambient type declaration files from jsdoc comments. Not sure if jsdoc is able to express all the features of typescript.

I could hand-code these declaration files, but keeping source and declaration file would be a pain as code evolves. What do you think of porting use-analytics into TS?

@orta
Copy link

orta commented May 21, 2020

If tsd-jsdoc is an issue, you can also have TypeScript generate .d.ts files from JSDoc:
http://www.staging-typescript.org/docs/handbook/declaration-files/dts-from-js.html

@DavidWells
Copy link
Owner

Hey @xthilakx

The way that it works with analytics-core is like this:

  1. This npm script is run
  2. during that script JSdocs are parsed and typedefs are generated
  3. Then a lil helper script is run to format the types into the final form. This last step is custom because of how analytics exports itself.

I would like to evolve this and make it easier to do, I'm not sure we can make the jump to full on typescript at this moment though.


A couple of options

  1. We can use a similar approach analytics-core uses to generate a types file
  2. We can include a hardcoded types.d.ts & copy that into the use-analytics lib folder on build
  3. Leverage @types to do this?
  4. Port to typescript (not something I can commit to right now)

I think #1 would be best to make sure types are always in sync. #2 would also work as the API shouldn't really change too much


@orta Interesting! I didn't know this was a thing. Do you know of any example projects I can have a look at that are using this approach?

@thilak-rao
Copy link
Contributor Author

thilak-rao commented May 21, 2020

I would like to evolve this and make it easier to do, I'm not sure we can make the jump to full-on typescript at this moment though.

@DavidWells How about we just port use-analytics into TypeScript. That package seems to be using microbundle to build, so porting should be as easy as adding a tsconfig file, renaming files, and annotating types.

I would be happy to open a PR if that's cool with you.

@DavidWells
Copy link
Owner

I'd rather not maintain a typescript package.

Happy to accept a PR with the types tho.

analytics@0.5.2 + has full types for the underlying library.

@saiichihashimoto
Copy link

analytics doesn't export its AnalyticsPlugin, so it's not really full types.

@DavidWells
Copy link
Owner

Hey @saiichihashimoto good point.

Do you know what the updated types should look like to export the AnalyticsPlugin as well?

@saiichihashimoto
Copy link

I'm not sure how tsd-jsdoc works, but I saw that the generated types have AnalyticsPlugin in there, just not exported.

@grumpyoldman-io
Copy link
Contributor

grumpyoldman-io commented Jul 12, 2023

For those here looking for the proper types, this should cover the entire package:

declare module 'use-analytics' {
  import type { ComponentType, Context, FC, ReactNode } from 'react';
  import type { AnalyticsInstance } from 'analytics';

  export function withAnalytics<P extends object>(Component: ComponentType<P>): FC<P>;

  export function useAnalytics(): AnalyticsInstance;
  export function useTrack(): AnalyticsInstance['track'];
  export function usePage(): AnalyticsInstance['page'];
  export function useIdentify(): AnalyticsInstance['identify'];

  export const AnalyticsConsumer: Context<AnalyticsInstance>['Consumer'];
  export const AnalyticsContext: Context<AnalyticsInstance>;

  export function AnalyticsProvider(props: {
    instance: AnalyticsInstance;
    children: ReactNode;
  }): JSX.Element;
}

I've also opened a PR to simply hardcode this in the package for now #389

@grumpyoldman-io grumpyoldman-io linked a pull request Jul 12, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants