Skip to content

Conversation

@adam-maj
Copy link
Contributor

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Dec 12, 2022

🦋 Changeset detected

Latest commit: 729e813

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@thirdweb-dev/wallets Minor
@thirdweb-dev/auth Major
@thirdweb-dev/react-core Minor
@thirdweb-dev/unity-js-bridge Minor
@thirdweb-dev/react Minor
@thirdweb-dev/sdk Minor
thirdweb Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Member

@joaquim-verges joaquim-verges left a comment

Choose a reason for hiding this comment

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

@adam-maj i would also rename
AbstractSigner -> AbstractWallet
GenericSignerWallet -> GenericAuthWallet (open to other names)

but need to make it clear cut when we use Signers (privatekey holders) vs Wallets (signer holders) - the hierarchy needs to be clear

@adam-maj
Copy link
Contributor Author

/release-pr

@jnsdls
Copy link
Member

jnsdls commented Jan 26, 2023

@adam-maj RE the conflicts, almost all of these files now live in react-core package (exception is the connect wallet component I believe)

@adam-maj
Copy link
Contributor Author

/release-pr

@adam-maj
Copy link
Contributor Author

/release-pr

Comment on lines +37 to +39
asyncHandler((req: Request, res: Response) =>
loginHandler(req, res, ctx as ThirdwebAuthContext),
),

Check failure

Code scanning / CodeQL

Missing rate limiting

This route handler performs [authorization](1), but is not rate-limited.
@adam-maj
Copy link
Contributor Author

/release-pr

@adam-maj
Copy link
Contributor Author

@joaquim-verges PR should be ready to go and tested everything on express/react.

Here are the versions: @thirdweb-dev/[email protected] @thirdweb-dev/[email protected] @thirdweb-dev/[email protected] @thirdweb-dev/[email protected]

Docs: https://docs-bypmstttd.thirdweb-preview.com/auth/server-frameworks/express

Things to test:

  • EVM & Solana wallets
  • Express & React
  • New Callbacks

@joaquim-verges
Copy link
Member

tested Next w/ EVM and Solana wallets - looking good from my side

"@thirdweb-dev/wallets": minor
---

Breaking change and upgrade to auth and minor update to related packages
Copy link
Member

Choose a reason for hiding this comment

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

@adam-maj we need some clean release notes here for auth - and noting separately any breaking change for each package.

remember that this shows up in the GH release notes

@@ -0,0 +1,613 @@
import type { Chain } from "@wagmi/core";

const arbitrum: Chain = {
Copy link
Member

Choose a reason for hiding this comment

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

any way we can use the Chains package for this?

@adam-maj
Copy link
Contributor Author

/release-pr

1 similar comment
@adam-maj
Copy link
Contributor Author

/release-pr

@adam-maj adam-maj merged commit a6c074c into main Jan 27, 2023
@adam-maj adam-maj deleted the am/auth branch January 27, 2023 22:07
@github-actions github-actions bot mentioned this pull request Jan 27, 2023
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.

4 participants