Skip to content

lists: PR #20 review nits — UnknownList comment + ListPreviewPane UX (F4/F6/F7) #23

@JamesCarnley

Description

@JamesCarnley

Follow-up from external review of #20 (review URL). NICE severity — devnet-shipping-acceptable; fold into next housekeeping pass.

F4 — Misleading "should never fire" comment

packages/hardhat/contracts/ListEntryResolver.sol:194:

```solidity
if (!d.exists) revert UnknownList(); // should never fire — onAttest ran first
```

Correct in normal EAS flow on a permanent resolver. False on a devnet ListEntryResolver redeploy: the new resolver has empty _decl, and a stale revoke of an entry attested under the previous resolver's storage hits this path. Mainnet resolvers are permanent per ADR-0030 so it never fires in production, but a reader of the comment trying to reason about devnet test scenarios gets the wrong invariant.

Suggested reword:

```solidity
if (!d.exists) revert UnknownList(); // unreachable when resolver state persists; may fire after a devnet resolver redeploy on stale revokes.
```

F6 — readEntryProperty returns null on missing dependencies

packages/nextjs/components/explorer/ListPreviewPane.tsx. The early-return at the start of readEntryProperty returns null when propertySchemaUID or other deps haven't loaded. The enrich() caller treats null as "no property set," so during the brief load window every entry renders as unordered/unlabeled (not as "loading"). The retain-last-known-value catch block doesn't fire because null isn't a throw.

Fix: throw a sentinel error instead of returning null so the catch path retains prior state.

```ts
if (!publicClient || !indexerAddress || !edgeResolverAddress || !propertySchemaUID || !easAddress)
throw new Error("readEntryProperty: deps not ready");
```

Devtools UX; not data corruption.

F7 — Drag-drop busy flag silently swallows concurrent drag

packages/nextjs/components/explorer/ListPreviewPane.tsx. busy is global to the pane and gates drop with no user feedback. The 3-attestation add can take several seconds; a drag mid-flight returns silently and the row snaps back without explanation. Add notification.info(\"Another operation in progress…\") on the busy-rejected path.

Refs

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions