Skip to content

Conversation

@joaquim-verges
Copy link
Member

@joaquim-verges joaquim-verges commented Oct 29, 2025


PR-Codex overview

This PR focuses on loosening the FacilitatorNetworkSchema in the packages/thirdweb/src/x402/schemas.ts file, changing it from a union of specific literals to a more general string type.

Detailed summary

  • Updated FacilitatorNetworkSchema from a union of specific network literals to z.string().
  • This change allows for a broader range of network identifiers.
  • FacilitatorNetwork type remains inferred from the updated schema.

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

Summary by CodeRabbit

  • Bug Fixes
    • Expanded network schema validation to accept a broader range of network identifiers, improving flexibility and compatibility.

@vercel
Copy link

vercel bot commented Oct 29, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
docs-v2 Ready Ready Preview Comment Oct 29, 2025 8:44am
nebula Ready Ready Preview Comment Oct 29, 2025 8:44am
thirdweb_playground Ready Ready Preview Comment Oct 29, 2025 8:44am
thirdweb-www Ready Ready Preview Comment Oct 29, 2025 8:44am
wallet-ui Ready Ready Preview Comment Oct 29, 2025 8:44am

@changeset-bot
Copy link

changeset-bot bot commented Oct 29, 2025

🦋 Changeset detected

Latest commit: cba9f79

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

This PR includes changesets to release 4 packages
Name Type
thirdweb Patch
@thirdweb-dev/nebula Patch
@thirdweb-dev/wagmi-adapter Patch
wagmi-inapp 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

@joaquim-verges joaquim-verges marked this pull request as ready for review October 29, 2025 08:30
@joaquim-verges joaquim-verges requested review from a team as code owners October 29, 2025 08:30
@github-actions github-actions bot added packages SDK Involves changes to the thirdweb SDK labels Oct 29, 2025
Copy link
Member Author


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • merge-queue - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

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

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@joaquim-verges joaquim-verges force-pushed the _X402_Simplify_FacilitatorNetworkSchema_to_accept_any_string branch from af9a0e7 to cba9f79 Compare October 29, 2025 08:30
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 29, 2025

Walkthrough

Added a patch release changeset. Modified the FacilitatorNetworkSchema in thirdweb package to accept any string value instead of a constrained union of specific network literals, removing prior network validation constraints.

Changes

Cohort / File(s) Summary
Release metadata
.changeset/five-plums-kiss.md
Added changeset declaring a patch release with description "Loosen network schema".
Network schema relaxation
packages/thirdweb/src/x402/schemas.ts
Replaced FacilitatorNetworkSchema from a union of specific network literals with eip155 prefix validation to z.string(), accepting any string value. Public type FacilitatorNetwork now infers from the string-based schema. All dependent schemas now accept any string for network parameter.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Primary concern: Verify the schema relaxation does not introduce security or validation gaps, particularly regarding how external systems depend on the constrained network values
  • Secondary concern: Ensure all dependent schemas that reference FacilitatorNetworkSchema have compatible downstream handling for arbitrary string values

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description Check ⚠️ Warning The pull request description is largely incomplete. While it includes the template placeholder comments, the author has not filled in the required sections specified in the description template. The description lacks a concrete PR title following the specified format, no issue tag or Linear reference (TEAM-0000 format) is provided, the "Notes for the reviewer" section is empty, and the "How to test" section contains no testing instructions. The only substantive content is an auto-generated PR-Codex summary, which does not fulfill the template requirements. The changeset file addition indicates this is a patch release, but no explanation of the rationale or impact of loosening the network schema is provided in the description. The author should fill out the pull request description template by providing: a properly formatted PR title (e.g., "[SDK] Fix: Loosen network schema in FacilitatorNetworkSchema"), the relevant Linear issue tag if applicable, notes for the reviewer explaining the rationale for changing from a constrained union to a plain string schema, and testing instructions or test results. Additionally, consider clarifying the implications of accepting any string value for the network field and whether this change is intentional or requires validation elsewhere.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "[X402] Simplify FacilitatorNetworkSchema to accept any string" directly and accurately describes the main change in the pull request. The changeset explicitly shows that FacilitatorNetworkSchema was transformed from a constrained union of specific network literals to a simple z.string() schema, which is exactly what the title conveys. The title is concise, specific, and would enable a teammate scanning the commit history to immediately understand the primary change without needing to review the detailed file changes.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch _X402_Simplify_FacilitatorNetworkSchema_to_accept_any_string

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • TEAM-0000: Entity not found: Issue - Could not find referenced Issue.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 931c730 and cba9f79.

📒 Files selected for processing (2)
  • .changeset/five-plums-kiss.md (1 hunks)
  • packages/thirdweb/src/x402/schemas.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from @/types or local types.ts barrels
Prefer type aliases over interface except for nominal shapes
Avoid any and unknown unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial, Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose

**/*.{ts,tsx}: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from @/types where applicable
Prefer type aliases over interface except for nominal shapes
Avoid any and unknown unless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size

Files:

  • packages/thirdweb/src/x402/schemas.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)

Files:

  • packages/thirdweb/src/x402/schemas.ts
packages/thirdweb/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

packages/thirdweb/**/*.{ts,tsx}: Every public symbol must have comprehensive TSDoc with at least one compiling @example and a custom tag (@beta, @internal, @experimental, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Lazy‑load heavy dependencies inside async paths (e.g., const { jsPDF } = await import("jspdf"))

Files:

  • packages/thirdweb/src/x402/schemas.ts
.changeset/*.md

📄 CodeRabbit inference engine (AGENTS.md)

.changeset/*.md: Each change in packages/* must include a changeset for the appropriate package
Version bump rules: patch for non‑API changes; minor for new/modified public API

Files:

  • .changeset/five-plums-kiss.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: Lint Packages
  • GitHub Check: E2E Tests (pnpm, esbuild)
  • GitHub Check: Size
  • GitHub Check: E2E Tests (pnpm, webpack)
  • GitHub Check: E2E Tests (pnpm, vite)
  • GitHub Check: Unit Tests
  • GitHub Check: Build Packages
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
.changeset/five-plums-kiss.md (1)

1-5: Verify the version bump level for this public API change.

While loosening the FacilitatorNetworkSchema constraint is backwards-compatible, it technically modifies a public API. The coding guidelines state "minor for new/modified public API" versus "patch for non‑API changes." Consider whether this should be a minor version bump instead of a patch.

As per coding guidelines.

Comment on lines +14 to 16
const FacilitatorNetworkSchema = z.string();

export type FacilitatorNetwork = z.infer<typeof FacilitatorNetworkSchema>;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Add required TSDoc and verify runtime validation coverage.

The FacilitatorNetworkSchema is a public symbol (exported via the FacilitatorNetwork type) but lacks TSDoc documentation. According to the coding guidelines, every public symbol in packages/thirdweb/** must have comprehensive TSDoc with at least one compiling @example and a custom tag.

Additionally, this change shifts validation from schema-time to runtime (via the networkToChainId function at lines 95-116). Ensure that all code paths consuming FacilitatorNetwork values properly call networkToChainId or otherwise validate the network string before use.

As per coding guidelines.

Consider adding TSDoc like:

+/**
+ * Schema for facilitator network identifiers.
+ * Accepts any string value; runtime validation via `networkToChainId` is required.
+ *
+ * @example
+ * ```ts
+ * const network = FacilitatorNetworkSchema.parse("eip155:1");
+ * const chainId = networkToChainId(network);
+ * ```
+ *
+ * @public
+ */
 const FacilitatorNetworkSchema = z.string();

Run the following script to verify that network values are validated before use:


🏁 Script executed:

#!/bin/bash
# Description: Find usages of FacilitatorNetwork or properties named 'network' from these schemas
# to verify runtime validation is applied

echo "=== Searching for FacilitatorNetwork type usage ==="
rg -n "FacilitatorNetwork" --type ts -C 3

echo -e "\n=== Searching for networkToChainId calls ==="
rg -n "networkToChainId" --type ts -C 2

echo -e "\n=== Searching for .network property access on x402 types ==="
rg -nP --type ts "\.network\b" -g '**/x402/**' -C 2

Length of output: 11966


Add required TSDoc—runtime validation coverage is properly handled.

The FacilitatorNetworkSchema and FacilitatorNetwork type lack TSDoc documentation, violating the coding guidelines requiring comprehensive TSDoc with @example and a custom tag for all public symbols in packages/thirdweb/**.

Runtime validation via networkToChainId() is properly applied across all critical code paths (sign.ts, fetchWithPayment.ts, common.ts).

Add TSDoc:

+/**
+ * Schema for facilitator network identifiers.
+ * Accepts any string value; runtime validation via `networkToChainId` is required.
+ *
+ * @example
+ * ```ts
+ * const network = FacilitatorNetworkSchema.parse("eip155:1");
+ * const chainId = networkToChainId(network);
+ * ```
+ *
+ * @public
+ */
 const FacilitatorNetworkSchema = z.string();

+/** @public */
 export type FacilitatorNetwork = z.infer<typeof FacilitatorNetworkSchema>;
🤖 Prompt for AI Agents
In packages/thirdweb/src/x402/schemas.ts around lines 14 to 16, the public
symbols FacilitatorNetworkSchema and FacilitatorNetwork are missing required
TSDoc; add a TSDoc block above FacilitatorNetworkSchema showing an example usage
(e.g., parsing "eip155:1" and passing to networkToChainId) and include the
@public tag, and add a short TSDoc with @public above the exported
FacilitatorNetwork type; ensure the comments follow existing project style and
include the example snippet shown in the review comment.

@github-actions
Copy link
Contributor

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
thirdweb (esm) 64.55 KB (0%) 1.3 s (0%) 232 ms (+284.09% 🔺) 1.6 s
thirdweb (cjs) 366.18 KB (0%) 7.4 s (0%) 638 ms (+36.1% 🔺) 8 s
thirdweb (minimal + tree-shaking) 5.73 KB (0%) 115 ms (0%) 111 ms (+3456.78% 🔺) 225 ms
thirdweb/chains (tree-shaking) 526 B (0%) 11 ms (0%) 96 ms (+7876.67% 🔺) 106 ms
thirdweb/react (minimal + tree-shaking) 19.09 KB (0%) 382 ms (0%) 100 ms (+2786.68% 🔺) 482 ms

@joaquim-verges joaquim-verges merged commit bcbb3f5 into main Oct 29, 2025
20 of 21 checks passed
@joaquim-verges joaquim-verges deleted the _X402_Simplify_FacilitatorNetworkSchema_to_accept_any_string branch October 29, 2025 09:11
@joaquim-verges joaquim-verges mentioned this pull request Oct 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

packages SDK Involves changes to the thirdweb SDK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants