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

Feature: allow arrow-function & class-methods as adapter factory #1197

Conversation

talentumtuum
Copy link
Contributor

Hi!
Now when implementing the "AdapterFactory" type from the package https://www.npmjs.com/package/@types/oidc-provider
a situation can happen where the types are correct, but at runtime we get an error.

I found a mention of this problem here #776 (comment)

With this PR I add the ability to use arrow-functions and class methods as a factory and at the same time fix the non-obvious behavior when writing code in typescript

@co1dmoon
Copy link

co1dmoon commented Jul 1, 2022

like

@talentumtuum talentumtuum force-pushed the feature/arrow-function-and-class-method-adapters branch from 2c474d3 to b863eb3 Compare July 1, 2022 09:47
@talentumtuum talentumtuum force-pushed the feature/arrow-function-and-class-method-adapters branch from b863eb3 to 4ac29be Compare July 1, 2022 10:35
@talentumtuum
Copy link
Contributor Author

@panva Ready for review

@panva
Copy link
Owner

panva commented Jul 4, 2022

@talentumtuum the code may pass initialization but static class function or arrow function factory functions don't actually work, i've added tests for such.

@talentumtuum
Copy link
Contributor Author

@panva I'll take care of this problem in my spare time, I think I'll be back with a solution in 1-3 days, maybe today. Okay?

@panva
Copy link
Owner

panva commented Jul 4, 2022

Thank you, for sure, come back to the problem any time. FWIW it might be as simple as updating base_model.js where the adapters get currently instantiated.

Edit: and client.js respectively too

@talentumtuum talentumtuum marked this pull request as draft July 4, 2022 11:05
@panva panva marked this pull request as ready for review July 4, 2022 14:02
@panva panva merged commit cee552f into panva:main Jul 4, 2022
@talentumtuum talentumtuum deleted the feature/arrow-function-and-class-method-adapters branch July 4, 2022 14:25
@talentumtuum
Copy link
Contributor Author

talentumtuum commented Jul 4, 2022

@panva Thank you for your quick assistance

@panva
Copy link
Owner

panva commented Jul 4, 2022

Thank you for the PR!

@github-actions github-actions bot locked and limited conversation to collaborators Oct 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants