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

Get rid of un-used dependencies #219

Closed
3 tasks done
CMCDragonkai opened this issue Jul 26, 2021 · 10 comments
Closed
3 tasks done

Get rid of un-used dependencies #219

CMCDragonkai opened this issue Jul 26, 2021 · 10 comments
Assignees
Labels
procedure Action that must be executed

Comments

@CMCDragonkai
Copy link
Member

CMCDragonkai commented Jul 26, 2021

A few dependencies we should be cleaning prior to merge:

  • conf
  • configstore

These ones are all about configuration? I'm pretty sure we are now using DB for most things, and just plain old JSON for others.

  • crypto-random-string

I'm pretty sure we replaced that with node-forge? And even then that might be replaced with native node in the future. Since mobile version of polykey will require a significant porting plan anyway for the native code.

  • express
  • express-openapi-validator
  • express-session
  • oauth2orize
  • passport
  • passport-http
  • passport-http-bearer
  • passport-oauth2-client-password

HTTP API doesn't yet exist, and we don't want to use express for an HTTP/OAuth API anyway. We would likely build that custom or use fastly.

  • js-yaml

What we are using YAML for??

  • jsonwebtoken

Isn't this replaced by jose?

  • pako

What are we using this for?

  • zxcvbn

We would want this later for password suggestion, but we haven't even built out our schema system... only then can we even use this.

And of course all the associated types for those libraries above.

Thoughts? @DrFacepalm @scottmmorris @joshuakarp

Tasks

  1. - Review above dependencies
  2. - Remove them
  3. - Update the repo
@CMCDragonkai CMCDragonkai added the procedure Action that must be executed label Jul 26, 2021
@joshuakarp
Copy link
Contributor

  • jsonwebtoken

This can be removed - only used in src-old, and we're using jose for JWTs instead.

@scottmmorris
Copy link
Contributor

pako is being used in gitUtils to compress some of the git information so it should stay

@CMCDragonkai
Copy link
Member Author

CMCDragonkai commented Jul 27, 2021 via email

@scottmmorris
Copy link
Contributor

But because we are writing our own custom handler for isomorphic git we should then include it. I just tried to run it without compression and it threw an error because the SHA check failed.

@CMCDragonkai
Copy link
Member Author

Changing this to @DrFacepalm as @joshuakarp has got a lot of issues in the current sprint.

@CMCDragonkai
Copy link
Member Author

I'm addressing this issue in https://gitlab.com/MatrixAI/Engineering/Polykey/js-polykey/-/merge_requests/206

In that MR I'll be removing all that is mentioned here.

The dependencies relating to the API may be used again in #166 which requires a proper design review.

@CMCDragonkai
Copy link
Member Author

@scottmmorris is uuid library needed? I see it being used for const vaultId = uuid();, but any randomly generated id should be sufficient.

See how we generate permission id:

async function generatePermId(): Promise<PermissionId> {
  const id = await keysUtils.getRandomBytes(32);
  return base58.encode(id) as PermissionId;
}

@CMCDragonkai
Copy link
Member Author

That also ensures that our random generators are the same and wherever else we need random ids.

@scottmmorris
Copy link
Contributor

@CMCDragonkai
Copy link
Member Author

Closing this issue as MR 206 merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
procedure Action that must be executed
Development

No branches or pull requests

5 participants