Skip to content

Conversation

@ElasticBottle
Copy link
Contributor

@ElasticBottle ElasticBottle commented Oct 30, 2024

Problem solved

Short description of the bug fixed or feature added


PR-Codex overview

This PR consolidates the custom JWT and custom authentication endpoint into a common endpoint, streamlining the authentication process in the thirdweb package.

Detailed summary

  • Removed getSessionHeaders function export in fetchers.ts.
  • Updated native-connector.ts to include ecosystem in authentication payloads.
  • Refactored guest authentication in guest.ts for clarity.
  • Replaced direct fetch calls with getClientFetch in jwt.ts and authEndpoint.ts.
  • Added ecosystem parameter to customJwt and authEndpoint functions.
  • Updated InAppWebConnector to handle recoveryCode in loginWithAuthToken.
  • Refactored authentication strategy handling to use consolidated endpoints.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@vercel
Copy link

vercel bot commented Oct 30, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 30, 2024 5:51am
thirdweb_playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 30, 2024 5:51am
thirdweb-www ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 30, 2024 5:51am
wallet-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 30, 2024 5:51am

@changeset-bot
Copy link

changeset-bot bot commented Oct 30, 2024

🦋 Changeset detected

Latest commit: 9425e9e

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

This PR includes changesets to release 1 package
Name Type
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

@graphite-app
Copy link
Contributor

graphite-app bot commented Oct 30, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 30, 2024

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
thirdweb (esm) 43 KB (0%) 861 ms (0%) 670 ms (-10.69% 🔽) 1.6 s
thirdweb (cjs) 102.91 KB (0%) 2.1 s (0%) 1.7 s (+17.84% 🔺) 3.7 s
thirdweb (minimal + tree-shaking) 4.84 KB (0%) 97 ms (0%) 45 ms (-9.18% 🔽) 142 ms
thirdweb/chains (tree-shaking) 498 B (0%) 10 ms (0%) 141 ms (+965.67% 🔺) 151 ms
thirdweb/react (minimal + tree-shaking) 17.38 KB (0%) 348 ms (0%) 198 ms (+32.16% 🔺) 545 ms

@codecov
Copy link

codecov bot commented Oct 30, 2024

Codecov Report

Attention: Patch coverage is 0% with 18 lines in your changes missing coverage. Please review.

Project coverage is 45.29%. Comparing base (556e113) to head (49bbbaf).

Files with missing lines Patch % Lines
...irdweb/src/wallets/in-app/web/lib/web-connector.ts 0.00% 18 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5229   +/-   ##
=======================================
  Coverage   45.29%   45.29%           
=======================================
  Files        1067     1067           
  Lines       55341    55339    -2     
  Branches     3973     3973           
=======================================
  Hits        25068    25068           
+ Misses      29581    29579    -2     
  Partials      692      692           
Flag Coverage Δ *Carryforward flag
legacy_packages 65.68% <ø> (ø) Carriedforward from e57fadc
packages 40.31% <0.00%> (+<0.01%) ⬆️

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
...irdweb/src/wallets/in-app/web/lib/web-connector.ts 0.28% <0.00%> (+<0.01%) ⬆️

@ElasticBottle ElasticBottle added the merge-queue Adds the pull request to Graphite's merge queue. label Oct 30, 2024
Copy link
Contributor Author

ElasticBottle commented Oct 30, 2024

Merge activity

  • Oct 30, 1:43 AM EDT: The merge label 'merge-queue' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Oct 30, 1:47 AM EDT: A user added this pull request to the Graphite merge queue.
  • Oct 30, 2:07 AM EDT: A user merged this pull request with the Graphite merge queue.

…5229)

## Problem solved

Short description of the bug fixed or feature added

<!-- start pr-codex -->

---

## PR-Codex overview
This PR consolidates authentication methods in the `thirdweb` package by introducing a common endpoint for custom JWT and custom auth, enhancing code organization and reducing redundancy.

### Detailed summary
- Changed `getSessionHeaders` to no longer export it.
- Introduced `ecosystem` parameter in various functions.
- Replaced direct `fetch` calls with `getClientFetch` for consistency.
- Simplified the `loginWithAuthToken` method to accept an optional `recoveryCode`.
- Unified handling of authentication strategies in `InAppWebConnector`.

> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`

<!-- end pr-codex -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-queue Adds the pull request to Graphite's merge queue. packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants