Skip to content

Fix email({ allowMultiple: true }) format/parse round-trip for quoted commas#626

Merged
dahlia merged 1 commit intomainfrom
fix/issue-354-email-round-trip
Mar 18, 2026
Merged

Fix email({ allowMultiple: true }) format/parse round-trip for quoted commas#626
dahlia merged 1 commit intomainfrom
fix/issue-354-email-round-trip

Conversation

@dahlia
Copy link
Copy Markdown
Owner

@dahlia dahlia commented Mar 18, 2026

Summary

email({ allowMultiple: true }) could not round-trip values whose local parts contained quoted commas. format() joined addresses with a bare ,, producing output like "Doe, John"@example.com,x@example.com. While splitEmails() happened to handle this correctly due to its quote-tracking state machine, the bare comma join was fragile and not guaranteed to be unambiguous.

This PR changes format() to use , (comma-space) as the separator instead. This is safe because email addresses cannot start with whitespace, and parse() already trims each split result. It also matches the RFC 5322 conventional separator for address lists.

const parser = email({ allowMultiple: true });
const value = ['"Doe, John"@example.com', "x@example.com"];

// Before: "Doe, John"@example.com,x@example.com
// After:  "Doe, John"@example.com, x@example.com
parser.format(value);

Closes #354

Test plan

  • Added round-trip test for quoted commas in local parts ("Doe, John"@example.com)
  • Added round-trip test for escaped quotes with commas ("a\"b,c"@example.com)
  • Updated existing format test in valueparser.test.ts to expect comma-space separator
  • mise test passes across Deno, Node.js, and Bun

email({ allowMultiple: true }).format() now joins addresses with ", "
(comma-space) instead of bare ",", so quoted local parts containing
commas survive a format() → parse() cycle.

Close #354

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gemini-code-assist
Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@dahlia dahlia added this to the Optique 1.0 milestone Mar 18, 2026
@dahlia dahlia added the bug Something isn't working label Mar 18, 2026
@dahlia dahlia self-assigned this Mar 18, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.92%. Comparing base (8ae37e2) to head (1649271).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #626   +/-   ##
=======================================
  Coverage   94.92%   94.92%           
=======================================
  Files          38       38           
  Lines       17431    17431           
  Branches     4599     4600    +1     
=======================================
  Hits        16546    16546           
  Misses        873      873           
  Partials       12       12           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 87ae74bb-aba0-4cb3-b6e7-d7f72bdbbd29

📥 Commits

Reviewing files that changed from the base of the PR and between d247413 and 1649271.

📒 Files selected for processing (3)
  • CHANGES.md
  • packages/core/src/valueparser.test.ts
  • packages/core/src/valueparser.ts

Walkthrough

The pull request fixes an email formatting bug in the email({ allowMultiple: true }) parser. The issue was that multiple email addresses were being joined with a bare comma when formatted, causing round-trip failures for emails with quoted local parts containing commas (e.g., "Doe, John"@example.com). The fix changes the join separator from "," to ", " (comma followed by space) in the format method. Changes include a changelog entry documenting the fix, an updated test with new round-trip test cases, and a one-line code modification in the valueparser implementation.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: fixing email format/parse round-trip for quoted commas in allowMultiple mode.
Description check ✅ Passed The description is directly related to the changeset, explaining the problem, solution, and test plan for the email round-trip fix.
Linked Issues check ✅ Passed The PR successfully addresses issue #354 by changing format() to use comma-space separator, enabling round-trip parsing of quoted commas.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the email format/parse round-trip issue; no out-of-scope modifications were introduced.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-354-email-round-trip
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@dahlia dahlia merged commit 878e39e into main Mar 18, 2026
8 checks passed
@dahlia dahlia deleted the fix/issue-354-email-round-trip branch March 18, 2026 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

email({ allowMultiple: true }) does not round-trip values containing quoted commas

1 participant