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

feat(endpoints): centralised all endpoints with types #438

Conversation

Ansonhkg
Copy link
Collaborator

Description

This PR introduces a centralised location to store all our endpoints. They are typed, which allows us to easily switch to another endpoint version when we release new versions.

NOTE: A new PR should be made to remove version overriding and change the executeJs and pkpSign endpoints to v1.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

I will create another PR for all the tests which will be merged into #435, and they will be tested all together.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@Ansonhkg Ansonhkg self-assigned this Apr 24, 2024
Comment on lines +26 to +30
// Get the version override for a particular endpoint
// FIXME: We will remove this completly once v0.1 is deployed to all public networks
if (isNode()) {
versionOverride = process.env[`${params.endpoint.envName}`] || undefined;
}

Choose a reason for hiding this comment

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

what is the purpose of this currently? It looks like a local test override but not completely sure.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yea, it's local test override so we can test habanero, manzano and cayenne which are on v0 right now, and code freeze & develop on v1 now

Copy link
Collaborator

@glitch003 glitch003 left a comment

Choose a reason for hiding this comment

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

seems good to me! thx

V1 = '/v1',
}

export const LIT_ENDPOINT = {
Copy link

Choose a reason for hiding this comment

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

Could be beneficial to add a type annotation for this, so that any typos in this array are flagged by compiler here, rather than getting a 'path is type string | undefined' some where else in the code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right! will do that in a separate PR!

@DashKash54
Copy link
Collaborator

NOTE: A new PR should be made to remove version overriding and change the executeJs and pkpSign endpoints to v1.

Let's track it on Linear @Ansonhkg

Copy link
Collaborator

@DashKash54 DashKash54 left a comment

Choose a reason for hiding this comment

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

LGTM!

@Ansonhkg
Copy link
Collaborator Author

NOTE: A new PR should be made to remove version overriding and change the executeJs and pkpSign endpoints to v1.

Let's track it on Linear @Ansonhkg

feature/lit-3032-auth-unification-remove-all-v0-stuff-after-we-deployed-v1

@Ansonhkg
Copy link
Collaborator Author

NOTE: A new PR should be made to remove version overriding and change the executeJs and pkpSign endpoints to v1.

Let's track it on Linear @Ansonhkg

https://linear.app/litprotocol/issue/LIT-3032/[auth-unification]-remove-all-v0-stuff-after-we-deployed-v1

@Ansonhkg Ansonhkg merged commit 44afb5c into feature/lit-2958-auth-unification-ts-tests Apr 25, 2024
2 checks passed
@Ansonhkg Ansonhkg deleted the feature/lit-2960-auth-unification-lit-endpoints-map branch April 25, 2024 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants