3

I've written a singleton class in Typescript to avoid having the keep declaring/creating the same OktaAuth object namely: new OktaAuth({ issuer: appConfig.OKTA_ORG_URL }).

(By the way I did this as I assume this the best way to write such things)

So at the moment my code looks like

export class OktaAuthorisation {
    private static instance: OktaAuthorisation;

    private constructor() {
    }

    static getInstance(): OktaAuthorisation {
        if (!OktaAuthorisation.instance) {
            OktaAuthorisation.instance = new OktaAuthorisation();
        }

        return OktaAuthorisation.instance;
    }

    private authClient = new OktaAuth({ issuer: appConfig.OKTA_ORG_URL });

    async accessToken(): Promise<OktaAccessToken> {
        return await this.authClient.tokenManager.get("accessToken");
    }

    async getOktaUserEmail() {
        let oktaUser: OktaUser = await this.authClient.token.getUserInfo(await this.accessToken());

        return oktaUser.email
    }
}

My question is where should the line private authClient = new OktaAuth({ issuer: appConfig.OKTA_ORG_URL }); live? Where it is in the above or should I be setting that in the constructor?

I'm not sure whether it would be recreated each time I use it - which I do like:

const oktaAuth = OktaAuthorisation.getInstance();

const myUser = await oktaAuth.getOktaUserEmail()

Also, in general is the tidiest way to go about this? And if I don't use the constructor for anything I guess I can remove it?

Thanks.

userMod2
  • 8,312
  • 13
  • 63
  • 115
  • In either case, it won't be recreated each time you use it; it will only be recreated each time you create a new instance of the class. – Shaun Luttin Feb 18 '20 at 03:31

1 Answers1

2

Where possible, set class state in the field declarations not in the constructor. This helps because:

  1. it avoids accidental null/undefined errors,
  2. it helps human readers quickly determine default values,
  3. it decreases the number of lines of code.

Whether you set class state in the field declaration or in the constructor, that state will only be recreated each time you create a new instance of the class not each time you access the field.

All that being said... whether you set class state in the field declarations or in the constructor, the TypeScript compiler output sets it in the constructor (by default). That is because mainstream JavaScript/ECMAScript does not support field declarations (though plans to soon). Here is an example from the playground with the TypeScript on the left and the compiler output on the right.

enter image description here

Shaun Luttin
  • 133,272
  • 81
  • 405
  • 467
  • Ok so I won't set in the constructor as mentioned - makes sense - less code, much easier to read. By the way if the constructor is empty, I presume I can just remove it, so is it good pratice to leave it in. Also, the way I'm using `getInstance` for example `const user = OktaAuthorisation.getInstance().getOktaUserEmail();`. Is this a good way to write this, feels a little long winded, having to use `getInstance()` each time? – userMod2 Feb 18 '20 at 03:32
  • @userMod2 If you want the constructor to remain private, then leave the constructor present, otherwise the constructor by default is public. – Shaun Luttin Feb 18 '20 at 03:36
  • @userMod2 Regarding `OktaAuthorisation.getInstance().getOktaUserEmail()`, sure, that's a bit long winded; if you are using a singleton pattern, though, it is the canonical way to get the instance and to access one of its members. I'm afraid going further into a design conversation about singletons is beyond the scope of SO comments. – Shaun Luttin Feb 18 '20 at 03:39