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.

Sign up for GitHub

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

Jump to bottom

TypeScript port #48

Merged
merged 1 commit into from
Oct 22, 2020
Merged

TypeScript port #48

merged 1 commit into from
Oct 22, 2020

Conversation

nulltoken
Copy link
Contributor

@nulltoken nulltoken commented Oct 3, 2020 •
edited
Loading

Fix #35

PR Checklist

  • I have run npm test locally and all tests are passing.
  • I have added/updated tests for any new behavior.
  • If this is a significant change, an issue has already been created where the problem / solution was discussed: [N/A, or add link to issue here]

PR Description

  • Port code to TypeScript
  • Try and reduce the code changes to the strict minimum to ease review

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging d80d9d2 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging 23aa9c0 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging 562e9b6 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging b4d5287 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging 92b7e06 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging a5278f6 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging ae2bbc1 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging 684a577 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging e31f23c into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging 659bb2b into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging 8f04fec into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging 5399f2a into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging 50e4014 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging f7e91e3 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging 0ab49ea into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging 339d9a3 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging 5a1f7c5 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 4, 2020

This pull request introduces 1 alert when merging c3e4e97 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 4, 2020

This pull request introduces 1 alert when merging c845d5e into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 4, 2020

This pull request introduces 1 alert when merging d2b5d4a into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@nulltoken nulltoken requested a review from poveden October 4, 2020 11:28
nulltoken
nulltoken commented Oct 4, 2020
View reviewed changes
package.json Show resolved Hide resolved
nulltoken
nulltoken commented Oct 4, 2020
View reviewed changes
package.json Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Oct 4, 2020

This pull request introduces 1 alert when merging 4ec3a0b into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 4, 2020

This pull request introduces 1 alert when merging 2a3a2b9 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 4, 2020

This pull request introduces 1 alert when merging 0c9637c into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 4, 2020

This pull request introduces 1 alert when merging 5debe65 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 4, 2020

This pull request introduces 1 alert when merging 8cf482b into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 4, 2020

This pull request introduces 1 alert when merging ee60b97 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 4, 2020

This pull request introduces 1 alert when merging 5f82bf1 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 4, 2020

This pull request introduces 1 alert when merging f3fe3bd into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@nulltoken nulltoken changed the title [WIP] TypeScript port TypeScript port Oct 5, 2020
poveden
poveden requested changes Oct 9, 2020
View reviewed changes
.gitignore Show resolved Hide resolved
src/lib/types.ts Outdated
statusCode: number;
}

export type scopesOrTransform = string | string[] | jwtTransform;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be Pascal-cased?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

src/lib/types.ts Outdated

export type scopesOrTransform = string | string[] | jwtTransform;

export interface jwtTransform {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be Pascal-cased?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

src/oauth2-mock-server.ts Outdated
@@ -125,47 +126,44 @@ function parsePort(portStr) {
return port;
}

function parseJWK(filename) {
async function parseJWK(filename: string): Promise<JWK.Key> {
const jwkStr = fs.readFileSync(filename, 'utf8');
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are async now, then this call should also be (via promisify, I guess).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

src/oauth2-mock-server.ts Outdated
function parsePEM(filename) {
return {
async function parsePEM(filename: string): Promise<JWK.Key> {
const pem = fs.readFileSync(filename, 'utf8');
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are async now, then this call should also be (via promisify, I guess).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@lgtm-com
Copy link

lgtm-com bot commented Oct 9, 2020

This pull request introduces 1 alert when merging e5dbd04 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@lgtm-com
Copy link

lgtm-com bot commented Oct 9, 2020

This pull request introduces 1 alert when merging b2195a2 into c293990 - view on LGTM.com

new alerts:

  • 1 for Server-side URL redirect

@nulltoken nulltoken requested a review from poveden October 9, 2020 19:21
@nulltoken nulltoken merged commit afb42d8 into master Oct 22, 2020
@nulltoken nulltoken deleted the ntk/ts branch October 22, 2020 10:03
@nulltoken nulltoken mentioned this pull request Nov 10, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@poveden poveden poveden approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Typescript type definitions
2 participants
@nulltoken @poveden