-
-
Notifications
You must be signed in to change notification settings - Fork 11
feat: initialize waitlist package with TypeScript, Vitest, and configuration files #356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…nd add nanoid dependency
…fig, WaitlistEvent, and WaitlistAdapter
… validation, and referral system
…mprove validation
…types, and basic tests
…al email entry operations
…elated implementations
…eval, referral code generation, and link creation
…e email for entry identification
…on-existent emails and adjust tests accordingly
…utation, referee retrieval, and referral code generation
…ent subscription, emission, and error handling
There was a problem hiding this 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
🤖 Fix all issues with AI agents
In `@packages/waitlist/README.md`:
- Around line 178-184: The README's WaitlistConfig shows emailValidation as an
inline object type; update it to reference the actual named type used in code by
changing the property to emailValidation?: EmailValidationConfig; — edit the
WaitlistConfig definition (the emailValidation property) to use
EmailValidationConfig (imported from "@zap-studio/validation/email/types")
instead of the inline { allowPlus?: boolean; allowSubdomains?: boolean } object
so the docs match the implementation.
♻️ Duplicate comments (1)
packages/waitlist/README.md (1)
173-173: Address thegetPositionresponse type inconsistency.This was flagged in a previous review: the README documents
number | null, but the TypeScript contract reportedly usesnumber | undefined. Since JSON cannot representundefined, the documentation should align with the actual implementation and explicitly specify whethernullis used when no position exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@packages/logging/package.json`:
- Around line 40-47: The package.json exports block is missing per-subpath type
declarations so TypeScript bundler resolution fails for "./console"; update the
"exports" mapping to include a types entry for the "./console" subpath (pointing
to the compiled declaration file for the console entry, e.g. dist/console.d.mts)
in addition to its JS entry, while keeping the root "types" field intact;
specifically modify the exports entry for "./console" so it provides both the JS
module and the corresponding types file to satisfy bundler resolution.
In `@packages/logging/src/console.ts`:
- Around line 33-39: The ConsoleLogger.error signature requires the err
parameter but the ILogger interface marks err as optional; update
ConsoleLogger.error to accept err?: unknown (make the second parameter optional)
and adjust the implementation to handle undefined (e.g., conditionally pass err
or omit it when calling console.error or include it only when present) so the
method matches ILogger and callers that omit the error won't cause TypeScript
errors; locate the method by name ConsoleLogger.error in
packages/logging/src/console.ts and update its parameter list and console.error
invocation accordingly.
- Around line 25-31: The info() and warn() methods duplicate formatting logic —
change them to delegate to the existing log() method: have info(message,
context?) call this.log('info', message, context) and have warn(message,
context?) call this.log('warn', message, context) so the formatting/printing is
centralized in log(); ensure the parameter names and optional context are
preserved to match log()'s signature (and update any typings if needed).
In `@packages/logging/src/index.ts`:
- Around line 9-23: The ILogger interface's log method currently returns void
while info, warn, and error return void | Promise<void>; update the log method
signature in ILogger to return void | Promise<void> so implementations can be
asynchronous (i.e., change log(level: LogLevel, message: string, context?:
Partial<TContext>): void to return void | Promise<void>), and ensure any
implementing classes/functions (those implementing ILogger) are updated to
satisfy the new signature for log, info, warn, and error consistently.
There was a problem hiding this 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
🤖 Fix all issues with AI agents
In `@apps/docs/src/about.md`:
- Around line 30-32: Update the "@zap-studio Packages — Modern Primitives"
section to include discoverable links to each new package by adding a short
inline list or bullet list of the `@zap-studio/*` packages (e.g.,
`@zap-studio/events`, `@zap-studio/waitlist`, `@zap-studio/logging`) that point
to their respective docs; modify the content near the "### `@zap-studio` Packages
— Modern Primitives" heading to insert the list so readers can navigate directly
to package pages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/logging/src/console.ts`:
- Around line 13-29: This file intentionally uses console.* for the built-in
console logger, so add an explicit lint exception: place a file-level eslint
disable comment (e.g., /* eslint-disable no-console */) at the top of
packages/logging/src/console.ts (or update the ESLint override for the console
logger in the package config) so the switch handling in the Console logger (the
switch(level) block and its "info"/"warn"/"error" cases) is exempt from the
repo-wide no-console rule.
In `@packages/logging/tests/index.test.ts`:
- Around line 26-30: The tests restore spies in afterEach (logSpy, warnSpy,
errorSpy) but call vi.useRealTimers() only at the end of the test file which can
leak mocked timers if a test throws; move the vi.useRealTimers() call into the
same afterEach that restores the spies so fake timers are always restored, and
mirror this change for the other test block that currently restores spies at
lines 80-94 (i.e. ensure any afterEach that calls mockRestore also calls
vi.useRealTimers()).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@apps/docs/src/packages/logging/index.md`:
- Line 40: Replace the non‑breaking hyphen character in the string "third‑party"
with a standard ASCII hyphen so the phrase reads "third-party"; locate the
occurrence in the documentation text (the token "third‑party" on the line shown)
and update it to use the regular hyphen character to ensure consistent markdown
rendering.
- Around line 15-36: The docs mention the ConsoleLogger timestamp option but
don't show its format; update the ConsoleLogger docs to state that setting `{
timestamp: true }` emits ISO 8601 UTC timestamps and include a sample output
line demonstrating how it appears (reference ConsoleLogger and the timestamp
option and show a sample call like logger.info("Message") with the sample output
prefixed by level and timestamp, e.g., `[INFO] 2025-01-26T19:44:00.123Z
Message`) so readers see the exact format and placement.
- Around line 54-66: The example AppLogger class currently leaves write()
unimplemented; implement AppLogger.protected write(level: LogLevel, message:
string, context?: Partial<AppContext>, err?: Error) to build a structured
logEntry containing level, message, timestamp (ISO), context, and an error
object when err is present (name, message, stack), then forward it to the
backend (e.g., await loggingService.send(logEntry)) or at minimum emit it via
console.log(JSON.stringify(logEntry))); ensure the implementation uses the
existing AbstractLogger signature and preserves the optional err and context
handling.
In `@packages/events/package.json`:
- Around line 14-19: Remove the unrelated "waitlist" token from the package.json
keywords array to keep the package metadata accurate; locate the keywords array
(containing "events", "event-bus", "waitlist", "typescript") and delete the
"waitlist" entry (or replace it with a more relevant keyword like "pubsub" if
desired) so the package.json only lists appropriate keywords.
- Around line 57-64: The package.json exports map lacks explicit per-subpath
type entries; update the "exports" object to include a "types" condition for
each exported path (e.g., "." and "./types") before other conditions so
TypeScript resolves type definitions correctly, and keep the root "types" field
(types) in sync or remove it if redundant; modify the "exports" entries
referenced in the "exports" key and ensure the existing "main" and "module"
targets (index.mjs) still map to the correct JS files while pointing their
"types" conditions to the corresponding .d.mts/.types.mjs files.
In `@packages/events/src/index.ts`:
- Around line 106-119: The emit method currently awaits each handler
sequentially which limits throughput; modify emit<T extends EventKey<TEventMap>>
to invoke all handler functions from this.handlers[type] without awaiting in the
loop, collect their promises and use Promise.allSettled to run them in parallel,
then iterate the settled results and call this.logger?.error for any rejected
result (including the rejection reason and the event type) so individual handler
failures are still logged; reference the emit method, this.handlers, and
this.logger in your changes.
|
Before releasing the waitlist package, I'd like to do this in order and review carefully:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a new @zap-studio/waitlist package (server + client + adapters + referral/leaderboard logic), alongside new foundational @zap-studio/events and @zap-studio/logging packages, and expands @zap-studio/validation with email validation utilities. It also updates monorepo tooling (Turbo tasks, NYC coverage reporting), scaffolding templates, and docs-site pages to reflect the new packages.
Changes:
- Add new packages:
waitlist,events, andlogging(source, tests, configs, docs, licensing). - Add Standard Schema-compatible email validation utilities and exports in
@zap-studio/validation. - Update monorepo tooling/config: Turbo pipeline tasks for coverage reporting, pnpm catalogs, package templates, and docs navigation.
Reviewed changes
Copilot reviewed 102 out of 109 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| turbo/generators/templates/package/tsconfig.json.hbs | Template now enables strictNullChecks. |
| turbo/generators/templates/package/package.json.hbs | Package template switches to LICENSE file name. |
| turbo/generators/templates/package/jsr.json.hbs | JSR template switches to LICENSE file name. |
| turbo/generators/templates/package/LICENSE.hbs | Adds a LICENSE template for generated packages. |
| turbo/generators/templates/config/tsconfig.json.hbs | Config template now enables strictNullChecks. |
| turbo/generators/config.ts | Generator prompt improved; generated license filename changed to LICENSE. |
| turbo.json | Adds Turbo tasks for merging JSON coverage and reporting. |
| todo.json | Adds a repo-level TODO list (waitlist adapters/integration). |
| pnpm-workspace.yaml | Adds nanoid and nyc to catalogs; includes zod catalog entry. |
| packages/waitlist/vitest.config.ts | Adds Vitest config for waitlist package. |
| packages/waitlist/tsdown.config.ts | Adds tsdown build config for waitlist package. |
| packages/waitlist/tsconfig.json | Adds waitlist TypeScript config with strictNullChecks. |
| packages/waitlist/tests/utils.test.ts | Adds tests for referral code generation and position calculation. |
| packages/waitlist/tests/server.test.ts | Adds integration-style tests for WaitlistServer. |
| packages/waitlist/tests/server.behavior.test.ts | Adds behavior-focused tests for WaitlistServer. |
| packages/waitlist/tests/referral/index.test.ts | Adds tests for referral utilities. |
| packages/waitlist/tests/events/index.test.ts | Adds tests exercising EventBus behavior in waitlist context. |
| packages/waitlist/tests/client.test.ts | Adds tests for WaitlistClient RPC behavior. |
| packages/waitlist/tests/adapters/in-memory.test.ts | Adds tests for InMemoryAdapter. |
| packages/waitlist/src/utils.ts | Adds referral code generation helper (nanoid-based). |
| packages/waitlist/src/types/index.ts | Defines waitlist domain types and event payload map. |
| packages/waitlist/src/server.ts | Implements WaitlistServer with validation, referral tracking, leaderboard, position. |
| packages/waitlist/src/schemas.ts | Adds Zod schemas for RPC validation. |
| packages/waitlist/src/referral/types.ts | Defines referral types (ReferralKey, ReferralLink). |
| packages/waitlist/src/referral/index.ts | Adds referral logic helpers (score, referees, code/link creation). |
| packages/waitlist/src/leaderboard/types.ts | Defines leaderboard types and strategies. |
| packages/waitlist/src/leaderboard/index.ts | Implements scoring, position, and stable sorting. |
| packages/waitlist/src/contract.ts | Defines WaitlistService interface (adapter + join/leave/getPosition). |
| packages/waitlist/src/constants.ts | Adds default API prefix and strategy defaults. |
| packages/waitlist/src/client.ts | Implements WaitlistClient RPC wrapper using @zap-studio/fetch. |
| packages/waitlist/src/adapters/storage/types.ts | Defines WaitlistStorageAdapter interface. |
| packages/waitlist/src/adapters/storage/in-memory.ts | Adds an in-memory storage adapter with indexes. |
| packages/waitlist/package.json | Adds @zap-studio/waitlist package manifest and exports. |
| packages/waitlist/jsr.json | Adds JSR publish config for waitlist. |
| packages/waitlist/README.md | Adds package documentation and RPC contract description. |
| packages/waitlist/LICENSE | Adds license file to waitlist package. |
| packages/waitlist/.gitignore | Adds waitlist-specific ignore file. |
| packages/validation/tsconfig.json | Enables strictNullChecks. |
| packages/validation/src/email/types.ts | Adds email types + validation config. |
| packages/validation/src/email/standard.ts | Adds Standard Schema-compatible email validation implementation. |
| packages/validation/src/email/constants.ts | Adds multipart public suffix list for subdomain checks. |
| packages/validation/package.json | Switches to LICENSE; exports new email modules. |
| packages/validation/jsr.json | Exports new email modules; switches to LICENSE in publish include. |
| packages/validation/README.md | Adds notes about email validator limitations. |
| packages/validation/LICENSE | Adds license file to validation package. |
| packages/permit/tsconfig.json | Enables strictNullChecks. |
| packages/permit/src/types.ts | Adds optional logger to Permit config types. |
| packages/permit/src/index.ts | Routes warnings through optional logger instead of console.warn. |
| packages/permit/package.json | Switches to LICENSE; adds optional peer dep on @zap-studio/logging. |
| packages/permit/jsr.json | Switches to LICENSE in publish include. |
| packages/permit/LICENSE | Adds license file to permit package. |
| packages/logging/vitest.config.ts | Adds Vitest config for logging package. |
| packages/logging/tsdown.config.ts | Adds tsdown build config for logging package. |
| packages/logging/tsconfig.json | Adds TypeScript config for logging package. |
| packages/logging/tests/index.test.ts | Adds tests for ConsoleLogger. |
| packages/logging/src/index.ts | Adds logging interfaces + AbstractLogger. |
| packages/logging/src/console.ts | Adds ConsoleLogger implementation. |
| packages/logging/package.json | Adds @zap-studio/logging package manifest and exports. |
| packages/logging/jsr.json | Adds JSR publish config for logging. |
| packages/logging/README.md | Adds logging package docs. |
| packages/logging/LICENSE | Adds license file to logging package. |
| packages/logging/.gitignore | Adds logging-specific ignore file. |
| packages/fetch/tsconfig.json | Enables strictNullChecks. |
| packages/fetch/package.json | Switches to LICENSE. |
| packages/fetch/jsr.json | Switches to LICENSE in publish include. |
| packages/fetch/LICENSE | Adds license file to fetch package. |
| packages/events/vitest.config.ts | Adds Vitest config for events package. |
| packages/events/tsdown.config.ts | Adds tsdown build config for events package. |
| packages/events/tsconfig.json | Adds TypeScript config for events package. |
| packages/events/tests/index.test.ts | Adds EventBus tests (including logger error reporting). |
| packages/events/src/types.ts | Adds typed event bus types + optional logger typing. |
| packages/events/src/index.ts | Implements EventBus (on/once/off/emit/clear/listenerCount). |
| packages/events/package.json | Adds @zap-studio/events manifest and exports. |
| packages/events/jsr.json | Adds JSR publish config for events. |
| packages/events/README.md | Adds events package docs. |
| packages/events/LICENSE | Adds license file to events package. |
| packages/events/.gitignore | Adds events-specific ignore file. |
| package.json | Updates pnpm version hash; adds prepare hook; aligns nyc version with catalog. |
| configs/vitest-config/tsconfig.json | Enables strictNullChecks. |
| configs/tsdown-config/tsconfig.json | Enables strictNullChecks. |
| biome.jsonc | Updates Ultracite Biome preset paths. |
| apps/docs/src/packages/waitlist/index.md | Adds EventBus section + optional install guidance. |
| apps/docs/src/packages/logging/index.md | Adds logging package docs page. |
| apps/docs/src/packages/events/utilities.md | Adds EventBus utilities docs page. |
| apps/docs/src/packages/events/index.md | Adds EventBus docs overview page. |
| apps/docs/src/packages/events/handlers.md | Adds EventBus handlers docs page. |
| apps/docs/src/packages/events/error-handling.md | Adds EventBus error handling docs page. |
| apps/docs/src/introduction.md | Formatting updates; removes package table block. |
| apps/docs/src/index.md | Adds events/logging to docs landing; reorders validation section. |
| apps/docs/src/about.md | Replaces package table with short narrative paragraph. |
| apps/docs/.vitepress/config.mts | Adds sidebar/nav entries for events/logging docs sections. |
| README.md | Adds events listing; reorders validation section. |
| LICENSE | Adds root LICENSE file. |
| AGENTS.md | Adds Ultracite standards doc. |
| .zed/settings.json | Adds Zed editor settings for Biome formatting. |
| .vscode/settings.json | Adds VS Code settings for Biome formatting. |
| .nycrc.json | Adds NYC report configuration. |
| .gitignore | Adjusts ignore rules; removes todo.json ignore; stops ignoring editor dirs. |
| .github/copilot-instructions.md | Removes old Copilot instructions file. |
| .cursor/hooks.json | Adds Cursor hook to auto-run Ultracite fix after edits. |
| .claude/settings.json | Adds Claude hook config to auto-run Ultracite fix. |
| .claude/CLAUDE.md | Adds Ultracite standards doc for Claude. |
| .changeset/shaggy-masks-approve.md | Changeset for waitlist event payload alignment. |
| .changeset/quiet-llamas-rhyme.md | Changeset for events package release. |
| .changeset/fresh-panthers-warn.md | Changeset for permit optional logger hook. |
| .changeset/early-trainers-reflect.md | Changeset for events + waitlist feature release. |
| .changeset/bright-ligers-observe.md | Changeset for logging package release. |
| .changeset/add-email-presets-validation.md | Changeset for validation email schema feature. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| if (result.status === "rejected") { | ||
| this.logger?.error("EventBus: Handler error", result.reason, { | ||
| event: type, | ||
| }); | ||
| } |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ILogger.error() can return a Promise, but this call is not awaited/handled. If a logger implementation is async and rejects, this can create an unhandled rejection. Consider awaiting logger.error(...) (and swallowing failures), or wrapping in Promise.resolve(...).catch(...).
| async list(): Promise<EmailEntry[]> { | ||
| return await Promise.resolve(Array.from(this.entries.values())); | ||
| } | ||
|
|
||
| async listEmails(): Promise<Email[]> { |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
list() returns insertion order, which can diverge from createdAt order (e.g., when inserting historical entries). The README documents list() as createdAt ascending for deterministic behavior; consider sorting by createdAt (tie-break by email) before returning.
| - `PATCH {prefix}/referrals` | ||
| - Body: `{ key: string; patch: Partial<ReferralLink> }` | ||
| - Response: `ReferralLink` |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The README documents PATCH {prefix}/referrals as accepting key: string, but WaitlistClient.updateReferral() sends key as { referrer, referee }. Update this endpoint doc to match the actual payload shape so implementers don’t build an incompatible server.
| logger?.warn( | ||
| `Resource validation failed for ${String(resourceType)}: ${String(error)}` | ||
| ); |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switching from console.warn to logger?.warn makes these warnings completely silent unless a logger is provided. If warning output is still desired by default, consider falling back to console.warn (or documenting that warnings are now opt-in).
| "dependencies": { | ||
| "@zap-studio/validation": "workspace:*" | ||
| }, |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zap-studio/validation is listed as a runtime dependency of @zap-studio/events, but the package source doesn’t import it. Consider removing this dependency to reduce install size and avoid implying a coupling that doesn’t exist.
| logger?.warn( | ||
| `Policy evaluation error for ${String(resourceType)}.${String(action)}: ${String(error)}` | ||
| ); |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logger?.warn() may be async (void | Promise<void>), but the returned promise isn’t handled. If an async logger rejects, this can surface as an unhandled rejection. Consider wrapping the call with Promise.resolve(...).catch(...) (or otherwise explicitly ignoring failures).
| const scores = new Map<Email, number>(); | ||
| for (const entry of entries) { | ||
| scores.set(entry.email, computeReferralScore(entry.email, referrals)); | ||
| } | ||
|
|
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the number-of-referrals strategy, this computes each score by scanning the full referrals array per entry (computeReferralScore uses filter). That is O(entries × referrals) and can become expensive. Consider building a single referrer->count map in one pass over referrals, then assigning scores from that map.
| import z from "zod"; | ||
|
|
||
| const EmailSchema: z.ZodEmail = z.email(); |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Zod is imported as a default export here, but the codebase uses named import (import { z } from "zod"). With ESM, default import can break at runtime depending on TS/bundler settings. Align this to the named import style used elsewhere.
| it("returns deterministic code when seed provided", () => { | ||
| const code6 = generateReferralCode(6); | ||
| const code7 = generateReferralCode(7); |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test case name mentions a "seed", but the argument passed to generateReferralCode() is the code length. Renaming this test to reflect length (or adding a real seed parameter) would make the intent accurate and easier to understand.
| async update(email: Email, patch: Partial<EmailEntry>): Promise<EmailEntry> { | ||
| const existing = this.entries.get(email); | ||
| if (!existing) { | ||
| throw new Error("Entry not found"); | ||
| } |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update() allows patch.email, but the entry remains stored under the original map key. This can create inconsistent state (value.email != key) and break lookups/uniqueness guarantees. Consider rejecting email changes or re-keying and updating indexes when email changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/permit/src/index.ts (1)
374-379:⚠️ Potential issue | 🟡 MinorLogger call inside
catchis unguarded.If
logger.warn()throws synchronously, the exception escapes thecatchblock and propagates to the caller — the original error context is lost. The same applies to the catch block on line 424–428.This is a minor resilience concern since well-behaved loggers won't throw, but a defensive wrapper would make the error path truly safe.
🛡️ Defensive pattern
} catch (error) { - logger?.warn( - `Resource validation failed for ${String(resourceType)}: ${String(error)}` - ); + try { + logger?.warn( + `Resource validation failed for ${String(resourceType)}: ${String(error)}` + ); + } catch { /* swallow logger errors */ } return null; }Apply the same pattern at line 425.
🤖 Fix all issues with AI agents
In @.changeset/shaggy-masks-approve.md:
- Line 5: Update the changelog entry that currently says "Align waitlist event
payloads with the simplified EventBus error handling" to a more specific
description: state exactly which waitlist event payload fields changed (e.g.,
renamed/removed/added fields and their types), summarize what "simplified
EventBus error handling" now does for consumers (e.g., returns standardized
error codes, suppresses retries, or emits a single error envelope), and include
any migration or behavioral notes (e.g., clients must read new field names like
waitlistId -> id, handle errorEnvelope {code,message} instead of raw errors, or
update listeners to ignore deprecated fields). Reference the terms "waitlist
event payloads" and "EventBus error handling" in your expanded sentence and add
a brief bullet or parenthetical with migration steps so users can quickly update
integrations.
In `@apps/docs/src/packages/logging/index.md`:
- Line 49: The phrase "low‑level" in the docs uses a non‑breaking hyphen
(U+2011); replace it with a standard ASCII hyphen U+002D so the text reads
"low-level". Locate the occurrence near the reference to AbstractLogger (the
sentence containing "extend `AbstractLogger` and implement the low‑level
`write()` method") and update the hyphen character only, leaving surrounding
formatting and backticks intact.
In `@packages/events/package.json`:
- Around line 34-36: Remove the unused dependency entry "@zap-studio/validation"
from the "dependencies" object in packages/events/package.json because it's not
imported anywhere under packages/events/src/; edit the package.json to delete
the dependency key/value and then run your package manager (install or update
lockfile) to refresh lockfiles and ensure the dependency is fully removed from
the project.
In `@packages/events/src/index.ts`:
- Around line 106-122: The emit method is calling this.logger?.error(...)
without awaiting it, which can produce unhandled promise rejections because
ILogger.error may return a Promise; update the emit flow (in
packages/events/src/index.ts, function emit) to await or properly handle the
Promise returned by this.logger?.error for each rejected handler result (e.g.,
capture the return value, await it, and wrap in try/catch or collect logger
Promises and await Promise.all with error handling) so any async logger errors
are observed and handled.
In `@packages/logging/src/index.ts`:
- Around line 53-71: The public methods in AbstractLogger (log, info, warn,
error) currently call this.write() (or this.log()) but do not return its result,
which drops any Promise from async overrides and causes unhandled rejections;
update these methods to return the result of their internal calls (e.g., have
log return this.write(...), info/warn return this.log(...), and error return
this.write(...)) and ensure the write method signature in the class/interface
allows async implementations (void | Promise<void>) so async overrides can
propagate their Promise to callers; reference the methods log, info, warn,
error, write, and formatMessage when making the changes.
In `@packages/validation/src/email/constants.ts`:
- Line 4: Rename the mistyped exported constant MULTIPART_LTDS to MULTIPART_TLDS
and update all usages/imports accordingly (e.g., change imports in
packages/validation/src/email/standard.ts and any other modules referencing
MULTIPART_LTDS). Ensure the exported symbol name in constants.ts and every place
that imports or references it uses MULTIPART_TLDS so compilation and linting
errors are resolved.
In `@packages/validation/src/email/standard.ts`:
- Around line 85-95: The JSDoc block currently describing public behavior is
attached to the internal helper validateWithSchema; change it to a concise
one-liner that documents the helper's purpose (e.g., "Internal synchronous
validator created from EmailSchema") or remove the block entirely, and move the
existing detailed JSDoc to the public validateEmail function so that
validateEmail documents usage/examples while validateWithSchema only has an
internal description; ensure references to EmailSchema and
createSyncStandardValidator remain clear.
- Around line 132-134: The plus-address check currently uses email.includes("+")
and can be triggered by a '+' in the domain; update the check to only inspect
the local part by splitting the address at '@' (e.g., const local =
email.split('@')[0]) and then test effective.allowPlus against
local.includes('+'); change the conditional in the function that contains
effective.allowPlus to use the extracted local variable and keep the same return
shape ({ valid: false, error: "Plus addressing not allowed" }) when rejected.
In `@packages/waitlist/README.md`:
- Around line 197-203: Update the parameter names in the WaitlistStorageAdapter
docs to use "email" instead of "id" for methods that take Email-typed
parameters: specifically change the parameter name in update(id: Email, patch:
Partial<EmailEntry>) and delete(id: Email) to update(email: Email, ...) and
delete(email: Email) (and any other occurrences in the interface or nearby
examples) so implementers clearly see the argument is an email address rather
than a generic identifier.
In `@packages/waitlist/src/adapters/storage/in-memory.ts`:
- Around line 26-31: updateReferralIndex currently overwrites any existing entry
for the same referralCode, so add a collision guard: in
updateReferralIndex(EmailEntry) first lookup
this.referralIndex.get(entry.referralCode) and if an existing entry exists and
its id !== entry.id throw an error (or return a failure) instead of overwriting;
ensure callers create() and update() propagate/handle that error. Also change
the counterpart (removeReferralIndex) to only delete the referralCode if the
stored entry.id === entry.id to avoid removing another entry's mapping.
- Around line 127-133: The delete(email: Email) method currently calls
removeReferralIndex(existing) but leaves a stale entry in referralCountIndex;
update the delete implementation (in the InMemory storage adapter) to also
remove the referralCountIndex entry for the deleted email (e.g.,
referralCountIndex.delete(email)) after confirming existing, so that
getReferralCount(email) returns nothing for removed emails; ensure you reference
the same Email key and only delete the referral count when an existing entry was
found.
- Around line 58-64: The methods in the in-memory storage (e.g., create,
createReferral, findByEmail, list, delete and similar async public methods in
the class) are using unnecessary await Promise.resolve(...) before returning
values; remove the redundant await Promise.resolve(...) calls and return the
value directly (e.g., return entry, return result, return undefined) so the
async function still returns a Promise without the extra wrapping/await; update
all occurrences in those methods accordingly and leave internal synchronous
logic (like this.entries.set(...) and this.updateReferralIndex(...)) unchanged.
| "@zap-studio/waitlist": minor | ||
| --- | ||
|
|
||
| Align waitlist event payloads with the simplified EventBus error handling. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Enhance the changelog description with specific details.
The current description is vague and doesn't provide enough context for users reading the changelog. Consider being more specific about:
- What changed in the waitlist event payloads (structure, fields, types)?
- What the "simplified EventBus error handling" means for consumers?
- Any migration steps or behavioral changes users should be aware of?
Example improvement:
-Align waitlist event payloads with the simplified EventBus error handling.
+Update waitlist event payloads to use the new EventBus error handling model. Error information is now included in the `error` field instead of being nested in the payload structure.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Align waitlist event payloads with the simplified EventBus error handling. | |
| Update waitlist event payloads to use the new EventBus error handling model. Error information is now included in the `error` field instead of being nested in the payload structure. |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
In @.changeset/shaggy-masks-approve.md at line 5, Update the changelog entry
that currently says "Align waitlist event payloads with the simplified EventBus
error handling" to a more specific description: state exactly which waitlist
event payload fields changed (e.g., renamed/removed/added fields and their
types), summarize what "simplified EventBus error handling" now does for
consumers (e.g., returns standardized error codes, suppresses retries, or emits
a single error envelope), and include any migration or behavioral notes (e.g.,
clients must read new field names like waitlistId -> id, handle errorEnvelope
{code,message} instead of raw errors, or update listeners to ignore deprecated
fields). Reference the terms "waitlist event payloads" and "EventBus error
handling" in your expanded sentence and add a brief bullet or parenthetical with
migration steps so users can quickly update integrations.
| ## Custom Logger (step by step) | ||
|
|
||
| If you need to forward logs to a third-party service (Datadog, Sentry, custom | ||
| backend), extend `AbstractLogger` and implement the low‑level `write()` method. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Non-breaking hyphen in "low‑level".
Same issue previously fixed for "third‑party" — the hyphen between "low" and "level" appears to be U+2011 (non‑breaking hyphen) rather than a standard ASCII hyphen (U+002D).
🔤 Fix
-backend), extend `AbstractLogger` and implement the low‑level `write()` method.
+backend), extend `AbstractLogger` and implement the low-level `write()` method.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| backend), extend `AbstractLogger` and implement the low‑level `write()` method. | |
| backend), extend `AbstractLogger` and implement the low-level `write()` method. |
🤖 Prompt for AI Agents
In `@apps/docs/src/packages/logging/index.md` at line 49, The phrase "low‑level"
in the docs uses a non‑breaking hyphen (U+2011); replace it with a standard
ASCII hyphen U+002D so the text reads "low-level". Locate the occurrence near
the reference to AbstractLogger (the sentence containing "extend
`AbstractLogger` and implement the low‑level `write()` method") and update the
hyphen character only, leaving surrounding formatting and backticks intact.
| "dependencies": { | ||
| "@zap-studio/validation": "workspace:*" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
rg -rn "@zap-studio/validation" packages/events/src/Repository: zap-studio/monorepo
Length of output: 45
Remove the unused @zap-studio/validation dependency.
The @zap-studio/validation package is listed in dependencies but is not imported anywhere in packages/events/src/. This dependency should be removed from the dependencies object in package.json.
🤖 Prompt for AI Agents
In `@packages/events/package.json` around lines 34 - 36, Remove the unused
dependency entry "@zap-studio/validation" from the "dependencies" object in
packages/events/package.json because it's not imported anywhere under
packages/events/src/; edit the package.json to delete the dependency key/value
and then run your package manager (install or update lockfile) to refresh
lockfiles and ensure the dependency is fully removed from the project.
| async emit<T extends EventKey<TEventMap>>( | ||
| type: T, | ||
| payload: TEventMap[T] | ||
| ): Promise<void> { | ||
| const list = [...(this.handlers[type] ?? [])]; | ||
|
|
||
| const results = await Promise.allSettled( | ||
| list.map((fn) => Promise.resolve().then(() => fn(payload))) | ||
| ); | ||
| for (const result of results) { | ||
| if (result.status === "rejected") { | ||
| this.logger?.error("EventBus: Handler error", result.reason, { | ||
| event: type, | ||
| }); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unawaited async logger may cause unhandled rejections.
ILogger.error returns void | Promise<void>. On Line 117, this.logger?.error(...) is not awaited, so if the logger implementation is async and throws, the rejection goes unhandled.
♻️ Suggested fix
for (const result of results) {
if (result.status === "rejected") {
- this.logger?.error("EventBus: Handler error", result.reason, {
+ await this.logger?.error("EventBus: Handler error", result.reason, {
event: type,
});
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async emit<T extends EventKey<TEventMap>>( | |
| type: T, | |
| payload: TEventMap[T] | |
| ): Promise<void> { | |
| const list = [...(this.handlers[type] ?? [])]; | |
| const results = await Promise.allSettled( | |
| list.map((fn) => Promise.resolve().then(() => fn(payload))) | |
| ); | |
| for (const result of results) { | |
| if (result.status === "rejected") { | |
| this.logger?.error("EventBus: Handler error", result.reason, { | |
| event: type, | |
| }); | |
| } | |
| } | |
| } | |
| async emit<T extends EventKey<TEventMap>>( | |
| type: T, | |
| payload: TEventMap[T] | |
| ): Promise<void> { | |
| const list = [...(this.handlers[type] ?? [])]; | |
| const results = await Promise.allSettled( | |
| list.map((fn) => Promise.resolve().then(() => fn(payload))) | |
| ); | |
| for (const result of results) { | |
| if (result.status === "rejected") { | |
| await this.logger?.error("EventBus: Handler error", result.reason, { | |
| event: type, | |
| }); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@packages/events/src/index.ts` around lines 106 - 122, The emit method is
calling this.logger?.error(...) without awaiting it, which can produce unhandled
promise rejections because ILogger.error may return a Promise; update the emit
flow (in packages/events/src/index.ts, function emit) to await or properly
handle the Promise returned by this.logger?.error for each rejected handler
result (e.g., capture the return value, await it, and wrap in try/catch or
collect logger Promises and await Promise.all with error handling) so any async
logger errors are observed and handled.
| log(level: LogLevel, message: string, context?: Partial<TContext>): void { | ||
| this.write(level, this.formatMessage(level, message), context); | ||
| } | ||
|
|
||
| info(message: string, context?: Partial<TContext>): void | Promise<void> { | ||
| this.log("info", message, context); | ||
| } | ||
|
|
||
| warn(message: string, context?: Partial<TContext>): void | Promise<void> { | ||
| this.log("warn", message, context); | ||
| } | ||
|
|
||
| error( | ||
| message: string, | ||
| err?: TError, | ||
| context?: Partial<TContext> | ||
| ): void | Promise<void> { | ||
| this.write("error", this.formatMessage("error", message), context, err); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Promises from async write() overrides are silently dropped.
The ILogger interface declares all methods as returning void | Promise<void>, signaling that async loggers are supported. However, log(), info(), warn(), and error() in AbstractLogger all invoke this.write() (or this.log()) as fire-and-forget statements — none of them return the result. If a concrete subclass implements write() as async (or overrides log() to be async), the returned Promise is discarded, which means:
- Callers who
await logger.info(...)getundefined, not the actualPromise. - Rejections from async
write()become unhandled promise rejections instead of propagating to the caller.
🔧 Proposed fix — propagate the return value
- log(level: LogLevel, message: string, context?: Partial<TContext>): void {
- this.write(level, this.formatMessage(level, message), context);
+ log(level: LogLevel, message: string, context?: Partial<TContext>): void | Promise<void> {
+ return this.write(level, this.formatMessage(level, message), context);
}
info(message: string, context?: Partial<TContext>): void | Promise<void> {
- this.log("info", message, context);
+ return this.log("info", message, context);
}
warn(message: string, context?: Partial<TContext>): void | Promise<void> {
- this.log("warn", message, context);
+ return this.log("warn", message, context);
}
error(
message: string,
err?: TError,
context?: Partial<TContext>
): void | Promise<void> {
- this.write("error", this.formatMessage("error", message), context, err);
+ return this.write("error", this.formatMessage("error", message), context, err);
}Also widen write() to allow async implementations:
protected abstract write(
level: LogLevel,
message: string,
context?: Partial<TContext>,
err?: TError
- ): void;
+ ): void | Promise<void>;🤖 Prompt for AI Agents
In `@packages/logging/src/index.ts` around lines 53 - 71, The public methods in
AbstractLogger (log, info, warn, error) currently call this.write() (or
this.log()) but do not return its result, which drops any Promise from async
overrides and causes unhandled rejections; update these methods to return the
result of their internal calls (e.g., have log return this.write(...), info/warn
return this.log(...), and error return this.write(...)) and ensure the write
method signature in the class/interface allows async implementations (void |
Promise<void>) so async overrides can propagate their Promise to callers;
reference the methods log, info, warn, error, write, and formatMessage when
making the changes.
| if (!effective.allowPlus && email.includes("+")) { | ||
| return { valid: false, error: "Plus addressing not allowed" }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plus-address check inspects the entire email, not just the local part.
email.includes("+") will reject emails if the domain somehow contains a +. The check should be scoped to the local part for correctness.
Proposed fix
- if (!effective.allowPlus && email.includes("+")) {
+ if (!effective.allowPlus && email.split("@")[0]?.includes("+")) {
return { valid: false, error: "Plus addressing not allowed" };
}🤖 Prompt for AI Agents
In `@packages/validation/src/email/standard.ts` around lines 132 - 134, The
plus-address check currently uses email.includes("+") and can be triggered by a
'+' in the domain; update the check to only inspect the local part by splitting
the address at '@' (e.g., const local = email.split('@')[0]) and then test
effective.allowPlus against local.includes('+'); change the conditional in the
function that contains effective.allowPlus to use the extracted local variable
and keep the same return shape ({ valid: false, error: "Plus addressing not
allowed" }) when rejected.
| interface WaitlistStorageAdapter { | ||
| create(entry: EmailEntry): Promise<EmailEntry>; | ||
| createReferral(link: ReferralLink): Promise<ReferralLink>; | ||
| update(id: Email, patch: Partial<EmailEntry>): Promise<EmailEntry>; | ||
| updateReferral(key: ReferralKey, patch: Partial<ReferralLink>): Promise<ReferralLink>; | ||
| delete(id: Email): Promise<void>; | ||
| deleteReferral(key: ReferralKey): Promise<void>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Use email instead of id for Email-typed parameters in docs.
These adapter methods accept Email but label the parameter as id, which reads like a separate identifier. Renaming the parameter in docs avoids confusion for implementers.
🧾 Suggested doc tweak
-interface WaitlistStorageAdapter {
- create(entry: EmailEntry): Promise<EmailEntry>;
- createReferral(link: ReferralLink): Promise<ReferralLink>;
- update(id: Email, patch: Partial<EmailEntry>): Promise<EmailEntry>;
- updateReferral(key: ReferralKey, patch: Partial<ReferralLink>): Promise<ReferralLink>;
- delete(id: Email): Promise<void>;
- deleteReferral(key: ReferralKey): Promise<void>;
+interface WaitlistStorageAdapter {
+ create(entry: EmailEntry): Promise<EmailEntry>;
+ createReferral(link: ReferralLink): Promise<ReferralLink>;
+ update(email: Email, patch: Partial<EmailEntry>): Promise<EmailEntry>;
+ updateReferral(key: ReferralKey, patch: Partial<ReferralLink>): Promise<ReferralLink>;
+ delete(email: Email): Promise<void>;
+ deleteReferral(key: ReferralKey): Promise<void>;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| interface WaitlistStorageAdapter { | |
| create(entry: EmailEntry): Promise<EmailEntry>; | |
| createReferral(link: ReferralLink): Promise<ReferralLink>; | |
| update(id: Email, patch: Partial<EmailEntry>): Promise<EmailEntry>; | |
| updateReferral(key: ReferralKey, patch: Partial<ReferralLink>): Promise<ReferralLink>; | |
| delete(id: Email): Promise<void>; | |
| deleteReferral(key: ReferralKey): Promise<void>; | |
| interface WaitlistStorageAdapter { | |
| create(entry: EmailEntry): Promise<EmailEntry>; | |
| createReferral(link: ReferralLink): Promise<ReferralLink>; | |
| update(email: Email, patch: Partial<EmailEntry>): Promise<EmailEntry>; | |
| updateReferral(key: ReferralKey, patch: Partial<ReferralLink>): Promise<ReferralLink>; | |
| delete(email: Email): Promise<void>; | |
| deleteReferral(key: ReferralKey): Promise<void>; |
🤖 Prompt for AI Agents
In `@packages/waitlist/README.md` around lines 197 - 203, Update the parameter
names in the WaitlistStorageAdapter docs to use "email" instead of "id" for
methods that take Email-typed parameters: specifically change the parameter name
in update(id: Email, patch: Partial<EmailEntry>) and delete(id: Email) to
update(email: Email, ...) and delete(email: Email) (and any other occurrences in
the interface or nearby examples) so implementers clearly see the argument is an
email address rather than a generic identifier.
| private updateReferralIndex(entry: EmailEntry): void { | ||
| if (!entry.referralCode) { | ||
| return; | ||
| } | ||
| this.referralIndex.set(entry.referralCode, entry); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
referralIndex silently overwrites on duplicate referralCode across entries.
updateReferralIndex unconditionally sets the index entry. If two different EmailEntry objects share the same referralCode, create() (and update()) will silently overwrite the index, making the first entry unreachable via findByReferralCode. Consider adding a collision guard.
🛠️ Proposed fix in `updateReferralIndex`
private updateReferralIndex(entry: EmailEntry): void {
if (!entry.referralCode) {
return;
}
+ const existing = this.referralIndex.get(entry.referralCode);
+ if (existing && existing.email !== entry.email) {
+ throw new Error(`Referral code ${entry.referralCode} is already in use`);
+ }
this.referralIndex.set(entry.referralCode, entry);
}Also applies to: 58-65
🤖 Prompt for AI Agents
In `@packages/waitlist/src/adapters/storage/in-memory.ts` around lines 26 - 31,
updateReferralIndex currently overwrites any existing entry for the same
referralCode, so add a collision guard: in updateReferralIndex(EmailEntry) first
lookup this.referralIndex.get(entry.referralCode) and if an existing entry
exists and its id !== entry.id throw an error (or return a failure) instead of
overwriting; ensure callers create() and update() propagate/handle that error.
Also change the counterpart (removeReferralIndex) to only delete the
referralCode if the stored entry.id === entry.id to avoid removing another
entry's mapping.
| async create(entry: EmailEntry): Promise<EmailEntry> { | ||
| if (this.entries.has(entry.email)) { | ||
| throw new Error("Entry already exists"); | ||
| } | ||
| this.entries.set(entry.email, entry); | ||
| this.updateReferralIndex(entry); | ||
| return await Promise.resolve(entry); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Remove redundant await Promise.resolve(...) wrapping throughout.
Every public method is async, so returning a value directly already produces a Promise. The await Promise.resolve(value) pattern adds unnecessary overhead and noise. This applies to all methods in the class (create, createReferral, findByEmail, list, delete, etc.).
♻️ Example fix (apply analogously to all methods)
async create(entry: EmailEntry): Promise<EmailEntry> {
if (this.entries.has(entry.email)) {
throw new Error("Entry already exists");
}
this.entries.set(entry.email, entry);
this.updateReferralIndex(entry);
- return await Promise.resolve(entry);
+ return entry;
}
async findByEmail(email: Email): Promise<EmailEntry | null> {
- return await Promise.resolve(this.entries.get(email) ?? null);
+ return this.entries.get(email) ?? null;
}
async list(): Promise<EmailEntry[]> {
- return await Promise.resolve(Array.from(this.entries.values()));
+ return Array.from(this.entries.values());
}Also applies to: 67-74, 127-133, 147-148, 159-160
🤖 Prompt for AI Agents
In `@packages/waitlist/src/adapters/storage/in-memory.ts` around lines 58 - 64,
The methods in the in-memory storage (e.g., create, createReferral, findByEmail,
list, delete and similar async public methods in the class) are using
unnecessary await Promise.resolve(...) before returning values; remove the
redundant await Promise.resolve(...) calls and return the value directly (e.g.,
return entry, return result, return undefined) so the async function still
returns a Promise without the extra wrapping/await; update all occurrences in
those methods accordingly and leave internal synchronous logic (like
this.entries.set(...) and this.updateReferralIndex(...)) unchanged.
| async delete(email: Email): Promise<void> { | ||
| const existing = this.entries.get(email); | ||
| if (existing) { | ||
| this.removeReferralIndex(existing); | ||
| } | ||
| await Promise.resolve(this.entries.delete(email)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete() leaves a stale referralCountIndex entry for the deleted email.
When an entry is deleted, removeReferralIndex cleans up the referral code index, but if the deleted email has a non-zero referral count in referralCountIndex, that phantom entry persists. Subsequent getReferralCount() calls for the deleted email will return a stale count. Consider also deleting the referral count entry.
🛠️ Proposed fix
async delete(email: Email): Promise<void> {
const existing = this.entries.get(email);
if (existing) {
this.removeReferralIndex(existing);
}
+ this.referralCountIndex.delete(email);
await Promise.resolve(this.entries.delete(email));
}🤖 Prompt for AI Agents
In `@packages/waitlist/src/adapters/storage/in-memory.ts` around lines 127 - 133,
The delete(email: Email) method currently calls removeReferralIndex(existing)
but leaves a stale entry in referralCountIndex; update the delete implementation
(in the InMemory storage adapter) to also remove the referralCountIndex entry
for the deleted email (e.g., referralCountIndex.delete(email)) after confirming
existing, so that getReferralCount(email) returns nothing for removed emails;
ensure you reference the same Email key and only delete the referral count when
an existing entry was found.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores