Skip to content

Conversation

@sushilk123
Copy link
Contributor

Summary

Fixes critical inconsistencies in the Dutch locale implementation to match the structure and accuracy of other locales.

Issues Fixed

  • Missing verb property in Sizable object causing incorrect error message formatting
  • Inaccurate translations: "Te lang/Te kort" → "Te groot/Te klein" (matches English "Too big/Too small")
  • Inconsistent structure compared to other locale implementations (French, German, etc.)

Changes

  • Added verb: "te hebben" to all Sizable entries for consistency
  • Updated error messages to use proper Dutch translations
  • Improved message structure to properly utilize the verb property
  • Added comprehensive test coverage for Dutch locale

Testing

  • Added new test file nl.test.ts with coverage for key error scenarios
  • Verified consistency with existing locale patterns

Before/After

Before: "Te lang: verwacht dat string <=10 tekens bevat"
After: "Te groot: verwacht dat string te hebben <=10 tekens"

Fixes translation accuracy and maintains consistency with the established locale architecture.

- Add missing 'verb' property to Sizable object for consistency with other locales
- Fix error messages: 'Te lang/Te kort' -> 'Te groot/Te klein' to properly match English 'Too big/Too small'
- Update error message structure to use verb property correctly
- Add comprehensive test coverage for Dutch locale

Fixes inconsistencies with other locale implementations and improves translation accuracy.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 21, 2025

Walkthrough

This change introduces Dutch locale error message handling by adding test coverage and updating the locale implementation. A new test file validates four error scenarios for the Dutch locale—invalid type, size violations (too big/small), and format errors. The locale module itself expands its sizing metadata to include both a unit and a verb field, which are now propagated through the Sizable map and getSizing function. Error messages are adjusted to incorporate these verb values for more detailed messaging when sizing constraints are violated.

Pre-merge checks

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "fix(locales): improve Dutch (nl) localization" is fully aligned with the changes in the pull request. The changeset specifically adds improvements to the Dutch locale implementation by introducing a verb property to the Sizable object and updating error message translations, which is exactly what the title conveys. The title is concise, clear, and specific enough that a teammate scanning history would immediately understand the primary change involves Dutch locale improvements.
Description Check ✅ Passed The PR description is well-related to the changeset and provides meaningful context about the Dutch locale improvements. It specifically references the missing verb property in the Sizable object, the translation updates from "Te lang/Te kort" to "Te groot/Te klein", the addition of the new test file nl.test.ts, and includes a concrete before/after example. All of these points align directly with the actual changes in the code, and the description is neither vague nor off-topic.

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.

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: 0

🧹 Nitpick comments (3)
packages/zod/src/v4/core/tests/locales/nl.test.ts (1)

16-24: Consider reviewing Dutch grammar with a native speaker.

The phrase "verwacht dat string te hebben <=10 tekens" uses an infinitive construction that feels a bit unnatural in Dutch. More idiomatic alternatives might be:

  • "verwacht dat string <=10 tekens heeft" (finite verb)
  • "verwacht maximaal 10 tekens voor string"

The test correctly validates the current implementation, but you might want to verify the phrasing with a native Dutch speaker to ensure it sounds natural.

packages/zod/src/v4/locales/nl.ts (2)

80-86: Remove unnecessary nullish coalescing on sizing.unit.

If sizing exists, sizing.unit is always defined per your type definition. The ?? "elementen" fallback on line 84 is unnecessary.

Apply this diff:

-          return `Te groot: verwacht dat ${issue.origin ?? "waarde"} ${sizing.verb} ${adj}${issue.maximum.toString()} ${sizing.unit ?? "elementen"}`;
+          return `Te groot: verwacht dat ${issue.origin ?? "waarde"} ${sizing.verb} ${adj}${issue.maximum.toString()} ${sizing.unit}`;

87-95: Inconsistent handling of issue.origin fallback.

Line 84 uses issue.origin ?? "waarde" but lines 91 and 94 use just issue.origin without the fallback. For consistency, either both too_big and too_small should have the fallback, or neither should.

Consider applying this diff to align with line 84:

-          return `Te klein: verwacht dat ${issue.origin} ${sizing.verb} ${adj}${issue.minimum.toString()} ${sizing.unit}`;
+          return `Te klein: verwacht dat ${issue.origin ?? "waarde"} ${sizing.verb} ${adj}${issue.minimum.toString()} ${sizing.unit}`;
-        return `Te klein: verwacht dat ${issue.origin} ${adj}${issue.minimum.toString()} is`;
+        return `Te klein: verwacht dat ${issue.origin ?? "waarde"} ${adj}${issue.minimum.toString()} is`;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8e4739f and 62fe682.

📒 Files selected for processing (2)
  • packages/zod/src/v4/core/tests/locales/nl.test.ts (1 hunks)
  • packages/zod/src/v4/locales/nl.ts (2 hunks)
🔇 Additional comments (6)
packages/zod/src/v4/core/tests/locales/nl.test.ts (4)

1-2: Imports look good.

Clean and minimal—just what you need for testing the Dutch locale.


7-14: LGTM!

The invalid_type test case correctly validates the Dutch error message for type mismatches.


26-34: Good coverage of the exclusive bound case.

Nice that you're testing with inclusive: false to verify the > operator is used correctly in the message.


36-42: LGTM!

The invalid_format test case validates the email format error message correctly.

packages/zod/src/v4/locales/nl.ts (2)

6-11: Consistent structure across all sizable types.

Good that you've added the verb property to all entries. The type structure is clean and consistent.


13-15: LGTM!

The updated return type correctly reflects the new structure with both unit and verb properties.

- Add required input property to test cases to match TypeScript interface requirements
- Ensures tests pass type checking while maintaining test coverage
@colinhacks
Copy link
Owner

Love it, thanks! Merging now, should land soon.

@colinhacks colinhacks merged commit 2849df8 into colinhacks:main Oct 21, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants