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

Update index.d.ts #542

Merged
merged 1 commit into from
Oct 3, 2019
Merged

Update index.d.ts #542

merged 1 commit into from
Oct 3, 2019

Conversation

vanthome
Copy link
Contributor

@vanthome vanthome commented Oct 3, 2019

Add constructor to OIDCContext class.

For me this is necessary as I want to use it like this:

    app.use(async (ctx, next) => {
      if (!ctx.oidc) {
        Object.defineProperty(ctx, 'oidc', { value: new provider.OIDCContext(ctx) });
      }

which is impossible with the current typings.

Add constructor to  OIDCContext class
@panva
Copy link
Owner

panva commented Oct 3, 2019

Hi @vanthome, can you elaborate on the use of this a bit more?

@vanthome
Copy link
Contributor Author

vanthome commented Oct 3, 2019

Look here:

The constructor requires a ctx but this is not reflected in the typings.

@panva
Copy link
Owner

panva commented Oct 3, 2019

I'm aware of the constructor. I'm just asking if you could elaborate on the use of your own constructed context. I'm curious, never encountered it before.

@vanthome
Copy link
Contributor Author

vanthome commented Oct 3, 2019

The code is originally not from me but I think it's inspired by this:
https://github.com/panva/node-oidc-provider/blob/464ebab1d2fbb284f7ce126f43adaa627c31874e/lib/shared/context_ensure_oidc.js

@panva
Copy link
Owner

panva commented Oct 3, 2019

Oh man, i'm just curious why you need it (not the change in index.d.ts but the actuall use of it), that's all :)

@vanthome
Copy link
Contributor Author

vanthome commented Oct 3, 2019

Ahh, You mean the module?
We are building a novel e-Commerce platform based on node and uServices and it's the perfect match:
https://github.com/restorecommerce

@panva
Copy link
Owner

panva commented Oct 3, 2019

What's the use you have for this?

    app.use(async (ctx, next) => {
      if (!ctx.oidc) {
        Object.defineProperty(ctx, 'oidc', { value: new provider.OIDCContext(ctx) });
      }

@vanthome
Copy link
Contributor Author

vanthome commented Oct 3, 2019

we have a setup function that initialized the OIDC provider when the application starts up.

      setupOIDC({
        app: this.app,
        issuer,
        logger: this.logger,
        cfg: this.cfg
      });

And the previously quoted code is then executed in that setupOIDC function.

@panva panva merged commit a5621a4 into panva:master Oct 3, 2019
@panva
Copy link
Owner

panva commented Oct 3, 2019

I don’t get what that’s good for but anyway, thanks for your feedback and contribution.

@vanthome
Copy link
Contributor Author

vanthome commented Oct 3, 2019

great, will show you the whole thing when we published it. Can you then make a new release pls?

@panva
Copy link
Owner

panva commented Oct 3, 2019

I will do a release when there are other, more pressing, unreleased items or periodically on sunday when there are trivial updates such as this one.

@panva
Copy link
Owner

panva commented Oct 4, 2019

Released, thanks for your contribution.


Please consider supporting the development of this and my other packages.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants