💥 Stop making breaking changes

[amcdnl]

Dear Okta Developers and PMs:

This library along with the @okta/okta-signin-widget and @okta/okta-react have been version revving and introducing breaking changes ( for little to no reason ) quite frequently. As a developer, I expect libraries coming from a commercial product to be fairly stable and work together with their other libraries very nicely. At this point, I can't trust this library or the other libraries I have mentioned to work together at all without several iterations of add/remove/revert and this has forced me to lock my versions and not upgrade. I urge you to introduce a beta channel, make sure all the libraries work together before releasing and show a little empathy for the developers using this library.

Sincerely -

Ordinary developer who wants a easy upgrade path

[aarongranick-okta]

@amcdnl We always try to avoid breaking changes except on major version releases. However fixes for bugs may be considered "breaking" if developers have come to depend on broken behaviors. These we also try to push to major version changes. Can you share any details on specific changes which have caused issues?

[amcdnl]

Related: https://github.com/okta/okta-react/issues/118 and https://github.com/okta/okta-signin-widget/issues/1888

[kellengreen]

Agree 💯, in fact I was about to make the exact same ticket.

authState.isPending was suddenly removed last week without providing any rational or documentation on how best to move forward. While over in okta-react land, restoreOriginalUri was suddenly throw into the mix a few months ago, just for kicks.

While I'm all for making things better and removing technical debt. The frequency of these breaking changes makes working with these libraries incredibly frustrating.

[kriszheng-cashwerkz]

I have a similar complain about the breaking changes since 5.0.0 released. The project I am working on, based on Vue 3, has a custom router guard to handle auth redirect from Okta. I am not using okta-vue library as at the time I started the project, it did not support Vue 3.

In the router guard function, the authState.isAuthenticated used to work and suddenly I got function does not exist error after upgraded to 5.0.0. This breaking change is not in the CHANGELOG and now I have no idea how to fix it but had to downgrade to 4.9.0.

Also I checked the okta-vue library and the latest version is still using 4.8.0.

[jalvaydev]

I have also just gone through the same issue with the breaking changes. As a dev new to using Okta, it is disappointing to see the changes be introduced without the official docs being updated to reflect these changes especially since I am learning how to set everything up.

[shuowu]

@kriszheng-cashwerkz FYI, okta-vue starts to support Vue 3 since version 4.0. Also, please stay with okta-auth-js version 4.x before it officially supports okta-auth-js version 5 (https://github.com/okta/okta-vue/blob/master/package.json#L109).

[makbol]

Today I updated to 5.0.0 and now I'm not getting removed and expired events in callbacks authClient.tokenManager.on('removed', () => { ... })

[jchabotamica]

+1 I am even trying to use this new "start()" method. Getting this error and i don't even call isPending anywhere:

Also getting a similar isAuthenticated issue

[DanielJoyce]

If they are using SemVer, breaking changes are allowed on major version bumps. If its not being documented it should be.

[InfinityBMX]

@DanielJoyce That's not the point. The point is they're making breaking changes with no benefit or clear upgrade path. I'm migrating an old neglected app from okta-react 3/okta-auth-js 4 to okta-react 6/okta-auth-js 5 and I pretty much have to rewrite the entire auth portion because half the methods and exposed variables I used have been removed or renamed (with new options and quirks). Updating the major to show changes doesn't make them a good idea.

[DanielJoyce]

By semver, it's perfectly okay that major version bumps can drastically change APIs. That's what major version changes are for.

That said for a commercial project, they should have a better change document and upgrade documentation.

[InfinityBMX]

SemVer is a way of tagging updates. The complaint isn’t that they didn’t tag their changes appropriately. The complaint is that they keep pumping out breaking changes for no apparent benefit faster than they can fully document them. Then they retire old versions so you’re forced to update to stay current.

[kellengreen]

100% to this.

The complaint is that they keep pumping out breaking changes for no apparent benefit faster than they can fully document them.

I really don't mind breaking changes, but when they're largely undocumented outside of PR notes, it's a recipe for frustration.

[DanielJoyce]

Welp, I can now understand what you guys are seeing. Nothing is kept in sync. Hoboy.

It looks like when they released 6.0 they didn't rebuild auth.d.ts

VsCode says "getUserInfo()" doesn't exist, though it is referenced in the docs and exists in the typescript code. I think they need to fix their release build process.

[DanielJoyce]

This has been a super frustrating experience trying to consume this in a Typescript project. Nothing right now seems to fit together and work type-wise. I've tried different versions of okta-react and okta-auth-js and it's just not working. The types are just not consistent both between the projects and compared to what the docs day.

[DanielJoyce]

Putting "strict": true in tsconfig.json causes a lot of warnings/errors, not just type related ones, but ones such as "X is possibly undefined" and "X is possibly null"

switching to

"strict": true, "noImplicitAny": false,

This removes a bunch of the implicit any errors, leaving other possibly more concerning errors behind.

Running yarn build results in approximately 300 errors with this config. Some just need more typing, others seem to be legit logic bugs.

  // Revokes the access token for the application session
  async revokeAccessToken(accessToken?: AccessToken): Promise<object> { // should be Promise<object | null>
    if (!accessToken) {
      accessToken = (await this.tokenManager.getTokens()).accessToken as AccessToken;
      const accessTokenKey = this.tokenManager.getStorageKeyByType('accessToken');
      this.tokenManager.remove(accessTokenKey);
    }
    // Access token may have been removed. In this case, we will silently succeed.
    if (!accessToken) {
      return Promise.resolve(null);
    }
    return this.token.revoke(accessToken);
  }

Here the user would assume the promise never returns null, but the code often does.

Another example is OktaAuth.ts, where the storagemanager is built with StorageManagerOptions, these could be null, but the class itself assumes they never are. StorageManagerOptions are allowed to be null in OktaAuthOptions

class OktaAuth implements SDKInterface, SigninAPI, SignoutAPI {
  options: OktaAuthOptions;
  storageManager: StorageManager;
  transactionManager: TransactionManager;
  tx: TransactionAPI;
  idx: IdxAPI;
  userAgent: string;
  session: SessionAPI;
  pkce: PkceAPI;
  static features: FeaturesAPI;
  features: FeaturesAPI;
  token: TokenAPI;
  _tokenQueue: PromiseQueue;
  emitter: typeof Emitter;
  tokenManager: TokenManager;
  authStateManager: AuthStateManager;
  fingerprint: FingerprintAPI;
  _pending: { handleLogin: boolean };
  constructor(args: OktaAuthOptions) {
    this.options = buildOptions(args);
    const { storageManager, cookies, storageUtil } = this.options;
    this.storageManager = new StorageManager(storageManager, cookies, storageUtil);

export interface OktaAuthOptions extends CustomUrls {
...
  storageManager?: StorageManagerOptions;
...
}

But storage manager constructor assumes they are not null.

export default class StorageManager {
...

  constructor(storageManagerOptions: StorageManagerOptions, cookieOptions: CookieOptions, storageUtil: StorageUtil) {
    this.storageManagerOptions = storageManagerOptions;
    this.cookieOptions = cookieOptions;
    this.storageUtil = storageUtil;
  }
...

It does look like the code in storage manager tries to always provide some kind of override to ensure a key isn't null. But because of this type checking hole, it's up to the library developer to continue to enforce this invariant.

Also, this means the generated docs are wrong, because it says StorageManager must always be given a non-null options object, and yet the code in the library itself does otherwise.

This means the docs are misleading, they don't agree with each other. Because of these large 'escape hatches' in the type checking, you can't rely on the tsdoc, as the code quite literally does something not implied by the published type defs.

[indignant]

+1

Moving from 5.5.0 to 5.6.0 started throwing. Seems like an intentional change that's not backwards compatible.

[hassanlatif]

+1 We are migrating within the same major version revision from 3.1.0 to 3.2.2 and the underlying okta-auth-js jumps from 4.x.x to 5.x.x with breaking changes. At least all the different okta packages should follow a semantic release whereby an underlying major version upgrade is reflected on the higher level package.

[DanielJoyce]

We decided to implement our own support and moved to keycloak...

[ccarse]

It would be nice to at least get some acknowledgement from Okta that they think this is an issue.