Skip to content

feat(schema): log warnings for removeField missing field and invalid HASH path#26

Open
linbo328 wants to merge 1 commit into
redis-developer:mainfrom
linbo328:fix/warn-on-missing-field
Open

feat(schema): log warnings for removeField missing field and invalid HASH path#26
linbo328 wants to merge 1 commit into
redis-developer:mainfrom
linbo328:fix/warn-on-missing-field

Conversation

@linbo328
Copy link
Copy Markdown

Summary

Implemented two warnings requested in the issue:

  1. removeField: Logs a warning when the field is not found, including the field name and list of known fields.
  2. addField: Logs a warning when a path is provided for HASH storage type.

Changes

  • src/schema/schema.ts: Replaced // TODO: log a warning comments with actual console.warn calls.

This helps users diagnose typos in field names when calling removeField.

Closes #13

…id path for HASH storage

- Log warning with field name and known fields list when removeField is called with non-existent field
- Log warning when path is provided for HASH storage type

This helps diagnose typos in field names.

Closes redis-developer#13
@banker
Copy link
Copy Markdown
Contributor

banker commented May 22, 2026

Thanks @linbo328 — appreciate that this covers both #13 and #12 in one PR.

Before merging, could you add unit tests covering both warnings? Suggested cases:

For removeField (closes #13):

  • Warning is emitted when removing a field that doesn't exist, and the message includes the field name + the list of known fields.
  • Warning includes the empty-state correctly (e.g. Known fields: []) when the schema has no fields.
  • No warning is emitted when removing a field that does exist, and the field is actually removed.

For makeField HASH-path (closes #12):

  • Warning is emitted when a non-null path is supplied for a HASH-storage field, and the message includes the field name and the path value.
  • No warning is emitted when path is null/undefined on HASH storage.
  • No warning is emitted on JSON storage where a path is legitimate.

Pattern: use vi.spyOn(console, 'warn').mockImplementation(() => {}) and assert with expect.stringContaining(...) on the message — see tests/unit/schema/schema.test.ts for the existing removeField test pattern, and PR #23 for a worked example of the spy-based assertion style.

Once tests are added I'll re-review and merge.

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.

Log a warning when removing a non-existent field

2 participants