📝 Add docstrings to feat/ensv2-sepolia
#1559
Conversation
Docstrings generation was requested by @shrugs. * #1516 (comment) The following files were modified: * `apps/ensapi/src/graphql-api/lib/get-domain-by-fqdn.ts` * `apps/ensindexer/src/lib/ponder-helpers.ts` * `packages/ensnode-sdk/src/registrars/basenames-subregistry.ts` * `packages/ensnode-sdk/src/registrars/ethnames-subregistry.ts` * `packages/ensnode-sdk/src/registrars/lineanames-subregistry.ts` * `packages/namehash-ui/src/utils/ensManager.ts` * `packages/namehash-ui/src/utils/ensMetadata.ts`
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
|
Important Review skippedThis PR was authored by the user configured for CodeRabbit reviews. By default, CodeRabbit skips reviewing PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback. To trigger a single review, invoke the You can disable this status message by setting the Comment |
Greptile OverviewGreptile SummaryThis PR adds comprehensive JSDoc docstrings to 7 functions across the codebase as requested by Key improvements:
Issues found:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Bot as CodeRabbit Bot
participant Code as Codebase
Dev->>Bot: Request docstrings generation (PR #1516)
Bot->>Code: Analyze functions needing documentation
Code-->>Bot: Return function signatures
Bot->>Bot: Generate JSDoc docstrings
Bot->>Code: Update 7 files with docstrings
Note over Code: get-domain-by-fqdn.ts<br/>ponder-helpers.ts<br/>basenames-subregistry.ts<br/>ethnames-subregistry.ts<br/>lineanames-subregistry.ts<br/>ensManager.ts<br/>ensMetadata.ts
Bot->>Code: Remove trailing newlines
Bot->>Code: Fix duplicate docstring marker
Bot->>Dev: Create PR #1559 with changes
Dev->>Dev: Review docstrings for accuracy
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| * @param namespaceId | ||
| * @returns registrar managed name | ||
| * @param namespaceId - The ENS namespace identifier to query | ||
| * @returns `"eth"` when `namespaceId` is `ENSNamespaceIds.Mainnet`, `ENSNamespaceIds.Sepolia`, `ENSNamespaceIds.SepoliaV2`, or `ENSNamespaceIds.EnsTestEnv`; `undefined` for other namespaces |
There was a problem hiding this comment.
Docstring says function returns undefined for other namespaces, but the function's return type is Name, not Name | undefined. The function actually returns undefined implicitly when none of the cases match.
| * @returns `"eth"` when `namespaceId` is `ENSNamespaceIds.Mainnet`, `ENSNamespaceIds.Sepolia`, `ENSNamespaceIds.SepoliaV2`, or `ENSNamespaceIds.EnsTestEnv`; `undefined` for other namespaces | |
| * @returns `"eth"` when `namespaceId` is `ENSNamespaceIds.Mainnet`, `ENSNamespaceIds.Sepolia`, `ENSNamespaceIds.SepoliaV2`, or `ENSNamespaceIds.EnsTestEnv`; `undefined` for all other namespaces |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ensnode-sdk/src/registrars/ethnames-subregistry.ts
Line: 40:40
Comment:
Docstring says function returns `undefined` for other namespaces, but the function's return type is `Name`, not `Name | undefined`. The function actually returns `undefined` implicitly when none of the cases match.
```suggestion
* @returns `"eth"` when `namespaceId` is `ENSNamespaceIds.Mainnet`, `ENSNamespaceIds.Sepolia`, `ENSNamespaceIds.SepoliaV2`, or `ENSNamespaceIds.EnsTestEnv`; `undefined` for all other namespaces
```
How can I resolve this? If you propose a fix, please make it concise.![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| * @param namespaceId | ||
| * @returns registrar managed name | ||
| * @param namespaceId - The ENS namespace identifier to query | ||
| * @returns `"eth"` when `namespaceId` is `ENSNamespaceIds.Mainnet`, `ENSNamespaceIds.Sepolia`, `ENSNamespaceIds.SepoliaV2`, or `ENSNamespaceIds.EnsTestEnv`; `undefined` for other namespaces |
There was a problem hiding this comment.
| * @returns `"eth"` when `namespaceId` is `ENSNamespaceIds.Mainnet`, `ENSNamespaceIds.Sepolia`, `ENSNamespaceIds.SepoliaV2`, or `ENSNamespaceIds.EnsTestEnv`; `undefined` for other namespaces | |
| * @returns `"eth"` for all ENS namespaces (`ENSNamespaceIds.Mainnet`, `ENSNamespaceIds.Sepolia`, `ENSNamespaceIds.SepoliaV2`, or `ENSNamespaceIds.EnsTestEnv`) |
The @returns docstring claims the function returns undefined for other namespaces, but ENSNamespaceId only has 4 possible values, all of which are handled by the switch statement.
View Details
Analysis
Misleading docstring in getEthnamesSubregistryManagedName() doesn't reflect actual function behavior
What fails: The JSDoc @returns description in getEthnamesSubregistryManagedName() claims the function returns undefined for "other namespaces", but ENSNamespaceId is a type union of exactly 4 values, all of which are handled by the switch statement, so the function always returns "eth" with no possibility of returning undefined.
How to reproduce: Examine the function definition at packages/ensnode-sdk/src/registrars/ethnames-subregistry.ts line 40. The docstring states: @returns \"eth"` when `namespaceId` is `ENSNamespaceIds.Mainnet`, `ENSNamespaceIds.Sepolia`, `ENSNamespaceIds.SepoliaV2`, or `ENSNamespaceIds.EnsTestEnv`; `undefined` for other namespaces`
However, examining packages/datasources/src/lib/types.ts, the ENSNamespaceId type is defined as: "mainnet" | "sepolia" | "sepolia-v2" | "ens-test-env" with no other possible values.
Result vs Expected: The docstring misleadingly documents a behavior (returning undefined for unmapped namespaces) that is impossible given the type system constraints. The function always returns "eth" regardless of input, since all 4 possible namespace values map to the same result.
Impact: Documentation mismatch makes the API contract unclear. Developers reading the docstring would expect the possibility of undefined returns when none exists. The function signature getEthnamesSubregistryManagedName(namespaceId: ENSNamespaceId): Name (without | undefined) is correct, but the docstring contradicts it.
Context: Similar functions like getBasenamesSubregistryManagedName() and getLineanamesSubregistryManagedName() correctly document that they throw errors for certain namespaces rather than returning undefined.
1 participant
Docstrings generation was requested by @shrugs.
The following files were modified:
apps/ensapi/src/graphql-api/lib/get-domain-by-fqdn.tsapps/ensindexer/src/lib/ponder-helpers.tspackages/ensnode-sdk/src/registrars/basenames-subregistry.tspackages/ensnode-sdk/src/registrars/ethnames-subregistry.tspackages/ensnode-sdk/src/registrars/lineanames-subregistry.tspackages/namehash-ui/src/utils/ensManager.tspackages/namehash-ui/src/utils/ensMetadata.tsThese file types are not supported
.changeset/short-cobras-bathe.mdpackage.jsonℹ️ Note