Skip to content
This repository has been archived by the owner on Jul 26, 2022. It is now read-only.

[okta-angular] Release 3.0.0 #778

Merged
merged 3 commits into from
Sep 10, 2020

Conversation

aarongranick-okta
Copy link
Contributor

@aarongranick-okta aarongranick-okta commented May 15, 2020

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Adding Tests
  • Build related changes
  • CI related changes
  • Documentation changes
  • Other... Please describe:

What is the current behavior?

Issue Number:
OKTA-283293
OKTA-290952

What is the new behavior?

Exposes the full AuthJS API via class inheritance.
Removes Typescript types for auth-js and configuration-validation
Updates internal test app to NG 7 (for Typescript > 3)
Updates okta-auth-js to 4.0.0 and configuration-validation to 1.0.0

Does this PR introduce a breaking change?

  • Yes
  • [] No

Reviewers

@swiftone
@shuowu

@swiftone
Copy link
Contributor

For cumulative changes we should have a PENDING entry in the CHANGELOG, so that nothing gets missed when we push out a version.

(I'm assuming the only reason this isn't getting an immediate release is because you have a few of these small changes going out at once)

@aarongranick-okta aarongranick-okta force-pushed the ag-angular-auth-instance-OKTA-283293 branch from ea6551c to a2c005e Compare July 23, 2020 01:12
@aarongranick-okta aarongranick-okta force-pushed the ag-angular-auth-instance-OKTA-283293 branch from 055cdbf to e291464 Compare September 2, 2020 01:43
@aarongranick-okta aarongranick-okta changed the title [okta-angular] Expose the internal AuthJS instance [okta-angular] Release 3.0.0 Sep 2, 2020
update authJS@4.0.0

updates @okta/configuration-validation@1.0.0

updates test app to ng@7

service inherits from okta-auth-js instance
@aarongranick-okta aarongranick-okta force-pushed the ag-angular-auth-instance-OKTA-283293 branch from 6755c59 to 3a134b2 Compare September 2, 2020 17:33
const tokens = res.tokens;
if (tokens.accessToken) {
this.oktaAuth.tokenManager.add('accessToken', tokens.accessToken as AccessToken);
this.tokenManager.add('accessToken', tokens.accessToken as AccessToken);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we export the token keys from okta-auth-js?

Shuo's other PR makes these keys configurable, which would break this.

Copy link
Contributor

@shuowu shuowu Sep 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this PR is only target to auth-js@4.0 instead of v4.1. My change goes into v4.1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    "@okta/okta-auth-js": "^4.0.0",

That includes 4.1, right?

My question about exporting the keys stands, either way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could modify the semver to ~4.0.0 but there should be no breaking changes in 4.1, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aarongranick-okta Right. I'm not saying you should change anything, I'm saying 4.1 should be free of breaking changes.

@shuowu shuowu removed the request for review from shuowu-okta September 2, 2020 18:13

> This library has been tested for compatibility with the following Angular versions: 4, 5, 6, 7, 8, 9
> :warning: Angular versions older than 7 may not be fully compatible with all dependencies of this library, due to an older Typescript version. You may be able to workaround this issue by setting `skipLibChecks: true` in your `tsconfig.json` file.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to claim compatibility with all of these older versions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is related to use of the "unknown" type within the typings for auth-js, "unknown" was introduced in Typescript 3.0. Angular 6 uses a 2.x version of Typescript so it complains. I don't think this breaks compatibility, but we might get questions about it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't talking about the details of that particular error, I'm just saying do we want to commit to supporting this set of versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I say let's wait until a user has an issue and see if we can update the docs with a simple workaround. If not, we can drop support. I don't think there are that many angular users who fail to upgrade - they should expect problems if using a very old version mixed with current versions of other modules.

@aarongranick-okta aarongranick-okta merged commit 41f4278 into master Sep 10, 2020
@aarongranick-okta aarongranick-okta deleted the ag-angular-auth-instance-OKTA-283293 branch September 10, 2020 16:26
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.

3 participants