facebook / facebook-nodejs-business-sdk

Node.js SDK for Meta Marketing APIs
https://developers.facebook.com/docs/business-sdk
Other
490 stars 226 forks source link

Need TypeScript definitions #130

Closed kevinclarkadstech closed 1 year ago

kevinclarkadstech commented 4 years ago

Which SDK version are you using?

"^5.0.1"

What's the issue?

Need TypeScript definitions.

Steps/Sample code to reproduce the issue

1) Create a .ts file 2) Type import fbSdk from 'facebook-nodejs-business-sdk';

Observed Results:

IDE throws an error: Could not find a declaration file for module 'facebook-nodejs-business-sdk'. '/Dev/facebook-nodejs-business-sdk-test/node_modules/facebook-nodejs-business-sdk/dist/cjs.js' implicitly has an 'any' type.
  Try `npm install @types/facebook-nodejs-business-sdk` if it exists or add a new declaration (.d.ts) file containing `declare module 'facebook-nodejs-business-sdk';`ts(7016)

Expected Results:

jookovjook commented 4 years ago

@kevinclarkadstech Types can be generated with dts-gen using it locally:

yarn add dts-gen
./node_modules/.bin/dts-gen -m facebook-nodejs-business-sdk -d types

/types/facebook-nodejs-business-sdk should be generated under the project root

Than add generated types to tsconfig.json:

{
  "compilerOptions": {
    ...
    "typeRoots": [
      ...
      "node_modules/@types",
      "./types"
      ]
  },
  "exclude": ["node_modules"]
}

Hope it will help : )

stale[bot] commented 4 years ago

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. Thank you for your contributions.

xeoncross commented 4 years ago

Would love to see TypeScript definitions maintained as well. How do you namespace or use the type definitions provided by dts-gen?

bduffany commented 4 years ago

The dts-gen solution mentioned by @jookovjook didn't work for me.

kevinclarkadstech commented 4 years ago

@kevinclarkadstech Types can be generated with dts-gen using it locally:


yarn add dts-gen

./node_modules/.bin/dts-gen -m facebook-nodejs-business-sdk -d types

/types/facebook-nodejs-business-sdk should be generated under the project root

Than add generated types to tsconfig.json:


{

  "compilerOptions": {

    ...

    "typeRoots": [

      ...

      "node_modules/@types",

      "./types"

      ]

  },

  "exclude": ["node_modules"]

}

Hope it will help : )

Thanks for responding...I just looked at the dts-gen library and noticed this:

This trade-off comes with a price -- you'll see a lot of anys in function parameters and return types. You may also see properties that are not intended for public use. dts-gen is meant to be a starting point for writing a high-quality definition file.

It would be better if Facebook devs would stop trying to create their own sht like Flow and just embrace TypeScript. I have heard from one popular Facebook developer that he never tried TypeScript. Just baffling.

jookovjook commented 4 years ago

@kevinclarkadstech Yeah. You'll see lots of warnings using this solution - it's not the best : ) Be prepared to use lot of @ts-ignore. But I think it's possible to make a fork of dts-gen which will work well with fb

simllll commented 4 years ago

It's been quite a while since this issue has been created, is there anything new regarding TS support?

kevinclarkadstech commented 4 years ago

It's been quite a while since this issue has been created, is there anything new regarding TS support?

There won't be...Facebook developers have not even tried TS, they just use Flow. πŸ‘Ž I mean this issue has been open for over 1/2 a year. Thank God I moved to a different project and don't need to use this library anymore. :wipes brow

simllll commented 4 years ago

@kevinclarkadstech what project do you use now? Is there any library which has ts support for the fb business sdk? I started to define some of the methods by using the dts-generated files as a basis, if we work together we could try to set something up in the https://github.com/DefinitelyTyped/DefinitelyTyped project.

noudadrichem commented 4 years ago

To what project did you move? Another FB SDK? @kevinclarkadstech

kevinclarkadstech commented 4 years ago

To what project did you move? Another FB SDK? @kevinclarkadstech

I actually moved to a new project on a new job :) So I did not need the SDK anymore.

kevinclarkadstech commented 4 years ago

@kevinclarkadstech what project do you use now? Is there any library which has ts support for the fb business sdk? I started to define some of the methods by using the dts-generated files as a basis, if we work together we could try to set something up in the https://github.com/DefinitelyTyped/DefinitelyTyped project.

Sorry, just seeing this now! Have you been using this SDK still?

I do think community supported type definitions would be better than none, but it’s pretty frustrating how against TS Facebook are. They keep using Flow. They always want to make everything for their little eco system (Yarn, Jest, Flow etc). Dan Abramov said he had never even tried TypeScript! Amusing to me because he also founded Redux, which is HORRIBLE without help from TS. πŸ˜…

Community supported type definitions usually work if a) the library is small or b) the library is big, but there are a lot of devs that use it and are willing to contribute. Otherwise it’s easy for them to get out of date.

Since you have looked at it more thoroughly, how hard does it seem to create full definitions?

jlubeck commented 4 years ago

While waiting on official definitions, I went ahead and created my own with dts-gen and I've been working nicely with it.

I ran into a problem though. I want to work with pagination now, and looking at the docs it mentions a Cursor object, which is in src/cursor.js. For some reason, this file was not exported by dts-gen

Anyone knows how I can work with it so I can use pagination in my calls with Typescript?

Thanks!

jlubeck commented 4 years ago

@simllll did you have to use Pagination yourself? Hopefully you are still using this library

Desnoo commented 4 years ago

@jlubeck I have a little example of how to use the cursor.

This will const cursor = igUser.getMedia(fields..., options, false) return a cursor. If you set the last argument to false it will return a cursor. Then you can call cursor.hasNext() to check if there is data for this API call. If there is data for this cursor you can get the data by calling cursor.next(). This will return a Promise.

I hope that helps.

jlubeck commented 4 years ago

hey @Desnoo thank you for your reply.

When creating my own definition, I changed the one I want from this:

getLeads(fields: any, ...args: any[]): any;

to this

getLeads(fields: any, ...args: any[]): Lead[];

That gave me all the code completion .

But with that, if I try:

let cursor = await ad.getLeads([],{},false);

I then get an error Property 'hasNext' does not exist on type 'Lead[]'

If I change the definition back to any, the hasNext doesn't error out, but how to I get the code completion back?

Desnoo commented 4 years ago

@jlubeck getLeads(fields: any, ...args: any[]): Lead[]; Well you defined the return type as Lead[] and not of the type Cursor.

The Type Cursor should look something like:

type Cursor<T> { hasNext(): boolean; next(): Promise<T>; }

This will be the return type if you pass false as the last argument to the getLeads(): Cursor<Lead[]> method. If you pass true or leave the last argument empty, then your definition should nearly work. The return type of getLeads then should be Promise<Lead[]>. I'm not completely sure, but it should look like this.

jlubeck commented 4 years ago

Thank you so Much @Desnoo with your help now I implemented the methods like this:

export type Cursor<T> = Array<T> & {
    hasNext(): boolean;
    next(): Promise<Cursor<T>>;
}

getLeads(fields: any, options: any, ignoreCursor: boolean): Promise<Cursor<Lead>>;

getLeads(fields: any, ...args: any[]): Promise<Lead[]>;

and works like charm!

abhijithvijayan commented 4 years ago

Setting the "typeRoots" property doesn't work. But setting "path" does.(See https://github.com/microsoft/TypeScript/issues/22217#issuecomment-369783776)

timofei-iatsenko commented 3 years ago

@jlubeck could you share your forked dts-gen and typings with all latest changes? Maybe we can then push it to DefinetelyTyped

jlubeck commented 3 years ago

@thekip I only update a couple that were what I was using... definitely not ready to push to a repo or something unfortunately

kevinclarkadstech commented 3 years ago

Just popped in to say sorry you guys are dealing with this still. I was actually looking at Facebooks NLP Node.js SDK today and OF COURSE....no TypeScript. So I went on to alternatives like Azure's LUIS. πŸ˜‘πŸ™„

anton-bot commented 3 years ago

Posting this comment to keep the issue open. Any reasonable project needs Typescript definitions.

kevinclarkadstech commented 3 years ago

Facebook seem to refuse to embrace TypeScript across the board. Don't hold your breath. Shit, they haven't even responded to this issue. 😑

nmggithub commented 3 years ago

I know people hate self-promo, and I don't know if this will work for most of you, but I've made a strongly-typed Facebook Graph API client (the nodes are typed as well). I have not tested it with ad-related endpoints, but it does work well for the Messenger Platform in my tests. Here is the link: https://github.com/nmggithub/fbsdk-ts

mptorz commented 3 years ago

Bumping up the issue

dcgudeman commented 3 years ago

Similar issue. Would love it if facebook would officially address it.

yonihod commented 3 years ago

I wonder if there is a good solution for this today?

noudadrichem commented 3 years ago

Probably not as FB is all in the 'flow' party.

kevinclarkadstech commented 3 years ago

I probably should just abandon this issue, it has been open 1.5 years with no response from anyone from facebook.

Arootin commented 3 years ago

Bumping up the issue

wtakayama-chwy commented 3 years ago

loading . . .

AlonMiz commented 3 years ago

loading . . .

connection timeout ⚠️

ugglr commented 3 years ago

Even if FB does not like typescript they should cater to their customers who actually use their advertising services. Now more than ever since apple basically has rendered "easy" use of their tracking obsolete. Not catering to typescript customers is the same as not supporting developers who are using Python or whatever.

If a company don't serve the costumers who are paying for their service then that's a company who will never have long term success.

davidpene commented 2 years ago

bump

alexblack commented 2 years ago

I'm hitting an issue trying to use dts-gen, has anyone else seen this? Thanks!

 ./node_modules/.bin/dts-gen -m facebook-nodejs-business-sdk -d types -o
Warning: Could not retrieve version/homepage information: HTTP Error 301: Moved Permanently for http://registry.npmjs.org/facebook-nodejs-business-sdk
nmggithub commented 2 years ago

I'm hitting an issue trying to use dts-gen, has anyone else seen this? Thanks!

 ./node_modules/.bin/dts-gen -m facebook-nodejs-business-sdk -d types -o
Warning: Could not retrieve version/homepage information: HTTP Error 301: Moved Permanently for http://registry.npmjs.org/facebook-nodejs-business-sdk

That seems like an issue with dts-gen and not this package.

bogdibota commented 2 years ago

That seems like an issue with dts-gen and not this package.

seems to be related to this PR: https://github.com/microsoft/dts-gen/pull/174

yesh-cash commented 2 years ago

I can't believe this project doesn't have typings. It's a shame really.

dima-gusyatiner commented 2 years ago

I was very disappointed to stumble upon this thread. I would go as far as to say this is a must in 2022.

achie27 commented 2 years ago

Consider this bumped

only4lee commented 2 years ago

I'd honestly settle if FB actually created swagger/OAS definitions. I do see one in FB's "incubating" github project that captures their conversions api, but it's unmaintained (at version 8 of the api). Using this I generated my own typescript library using the Open API code generator after modifying it to align to the current version (v12).

Works great -- only issue that it is problematic that FB appears to have chosen not to publish their APIs this way, at least meaningfully. It will be super annoying to maintain my own swaggers -- FB api docs are terrible. I've had to rage-quit (browsing API information) a few times when unable to find what I've needed.

kigorw commented 2 years ago

Bump! It's time to release types.

nmggithub commented 2 years ago

Facebook seems to really like their own type system, Flow. They probably will never release types.

jRexhmati commented 2 years ago

Crazy!!! Still no support for types, such a shame :/

K-Mistele commented 2 years ago

Ridiculous that they don't support this.

alexgrigore1 commented 2 years ago

I find it impossible to use this library because only some random features are documented and absolutely no autocomplete. I'll just go with rest api!

kevinclarkadstech commented 2 years ago

Just keep creating new issues instead of only responding to this. Flood their issues with Need TypeScript Types.

kevinclarkadstech commented 2 years ago

LOL from their README

"The Facebook Business SDK is a one-stop shop to help our partners better serve their businesses. Partners are using multiple Facebook API's to serve the needs of their clients. Adopting all these API's and keeping them up to date across the various platforms can be time consuming and ultimately prohibitive."

Yes, adopting these APIs without TypeScript types is TIME CONSUMING and PROHIBITIVE.

nusendra commented 2 years ago

bump