Thank you @orgads can you please make this independent from express? // cc @HappyZombies

branch v5-dev contains an entire TypeScript version. While this PR is fine, I think we need to be discussing Code Quality first, updating vulnerability packages and also (possibly) tackling what was left on v4. I think after all this THEN we can move to TS.

@jankapunkt Why? The Request/Response types accept express input.

@HappyZombies This PR is not about migration to TS. It merely adds the definitions file. This is required for users that use @types/oauth2-server. It's either adding the types here, or adding a new package to DefinitelyTyped.

@orgads I know they to but not everybody uses express (which is why there are the adapters). Wouldn't the current import force people to install express?

Not to mention that @types/oauth2-server is from the old/previous module, so things will probably be different once we start adding new features/differences. So it would be incorrect to use @types/oauth2-server since it's not related to this repository/package.

Also, per README -> "The @node-oauth/oauth2-server module is framework-agnostic". Making it the express Request basically means that the module now depends on Express.

@jankapunkt Done. Hope it's good enough now.

@HappyZombies This is exactly the reason I want to add the definitions file here, so it will be easier to adapt when making changes.

Generally, changes in a minor version should not break existing APIs, so the definitions file is not expected to change frequently.

@orgads got it, thanks for explaining, I am not 100% knowledgeable on the whole TypeScript-isms so thanks for being patient with me 😅

ping?

hey @orgads we are still setting this up, there are many things we have to get running first, before we can merge new features or fixes. Please follow on the following PRs:

#18 #32 #29 #30 #35

ping

Branch is still pointing to master, besides we will ping you once ready, so no need to keep bumping us.

Changed target to development and rebased.

Sorry for bothering, but I'm stuck with a vulnerable lodash dependency because of the original oauth2-server package.

Is anything holding you now from merging this and releasing a new version?

@orgads I activate CI for your PR and you need one more review from either @HappyZombies or @jwerre as I am not the typescript expert

Well?

If you add typings to your package, then you put them into the typings-attribute of package.json and not into the types-attribute.

Not according to the docs:

Note that the "typings" field is synonymous with types, and could be used as well.

yes you are right. I apologize,

@HappyZombies @jwerre I think this is now worth a second look since we released 4.1.0

This issue is still keeping me from switching to @node-oauth/oauth2-server (and probably many other).

I think this should be merged and released as soon as possible and improved later.

I lost my patience and created a forked package with the types: https://www.npmjs.com/package/@audc/oauth2-server

Feel free to use it :)

I'll try to get this out tomorrow, there were many issues initially so I was a bit sus, but I'm sure they're ironed out now. Sorry about that y'all.

Thank you.

@jwerre mind approving?

Ohhh... I'm not much of a Typescripter. I'll approve but not with any confidence.

Ohhh... I'm not much of a Typescripter. I'll approve but not with any confidence.

I'll verify the types on some existing projects, and update them if necessary.

@jwerre

I am a typescript user and I think that the types look Ok so far. We can always modify them if the are wrong. In worst case a typescript user would do a // @ts-ignore and report this as a finding.
So nobody should be blocked by the typings and they dont modify active code so no breaking change.

100% @Uzlopak , any serious issue I have can be fixed later on. Just wanna unblock people looking forward to this

I wonder it we could set up DTSLint in order to test the types?

Wie could use tsd like fastify does.

I added yesterday a type change to fastify/point-of-view and modified the test.

fastify/point-of-view#272 (comment)

Thanks for merging! Any idea when this will be released?

I'll try to get it out today or tomorrow

This has been released on 4.1.1