Skip to content

Conversation

@ricokahler
Copy link
Contributor

@ricokahler ricokahler commented Jun 3, 2025

This PR introduces two related changes to simplify the library's API and internal logic:

  1. Removes Console Warnings for undefined to null Conversion:
    The library previously logged a console warning when an undefined value within an array was converted to null during the diffing process. This warning has been removed. The conversion itself remains (as undefined is not valid JSON and null is the standard representation for "no value"), but it will no longer produce console output.
  2. Simplifies Internal APIs by Removing options Propagation:
    The hideWarnings option was the primary reason for propagating the options object deep into various internal diffing functions (diffItem, diffObject, diffArray, diffArrayByIndex, diffArrayByKey). With the removal of the warning and the hideWarnings option, this propagation is no longer necessary. These internal functions now have simpler signatures, no longer accepting or passing down the options object for this purpose.

Key Changes:

  • nullifyUndefined Function:
    • Removed the path, index, and options parameters.
    • Removed the conditional console.warn log.
    • The function now solely converts undefined to null, aligning its behavior more closely with JSON.stringify() without side effects.
    • Updated JSDoc to reflect this simplified behavior.
  • Removal of hideWarnings Option:
    • Removed the hideWarnings property from the PatchOptions interface.
    • Removed DEFAULT_OPTIONS constant as it's no longer needed.
  • Simplified Internal Function Signatures:
    • The options parameter has been removed from diffItem, diffObject, diffArray, diffArrayByIndex, and diffArrayByKey as it was primarily used for hideWarnings. These functions now rely on the default behavior for undefined handling.
  • Test Updates:
    • Removed hideWarnings: true from test calls to diffPatch.
    • Snapshots remain largely the same in terms of patch output, as the core diffing logic for values is unaffected, only the warning and option handling.

Rationale:

  • Cleaner Console Output: Removing the undefined to null conversion warning reduces noise in the console, especially for users who frequently work with data that might contain undefined values. The conversion to null is a standard and expected behavior when serializing to JSON-like structures.
  • Simplified API: Removing the hideWarnings option makes the public API slightly leaner.
  • Internal Code Simplification: Eliminating the need to pass the options object through multiple internal functions for the sole purpose of hideWarnings leads to cleaner and more maintainable internal code.
  • Alignment with JSON.stringify: The behavior of converting undefined in arrays to null is consistent with JSON.stringify, making the library's behavior more predictable for developers familiar with JSON standards.

This change streamlines the library's behavior regarding undefined values and simplifies its internal option handling.

Copy link
Contributor Author

ricokahler commented Jun 3, 2025

Copy link
Member

@rexxars rexxars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a hill I am willing to die on so will approve, but I don't agree that this was a necessary change 🤷‍♂️

The fact that JSON.stringify automatically converts undefined to null in arrays but not in objects has always seemed odd to me, and I'd rather have a warning about it than debugging why I am seeing nulls in my arrays.

@ricokahler ricokahler force-pushed the v6_remove-undefined-to-null-warnings branch from fc0b806 to e538df3 Compare June 6, 2025 21:57
@ricokahler ricokahler force-pushed the v6_remove-array-revision-lock-check branch from 3aff5f8 to 2459585 Compare June 6, 2025 21:57
Copy link
Contributor Author

ricokahler commented Jun 13, 2025

Merge activity

  • Jun 13, 7:24 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jun 13, 7:28 PM UTC: Graphite rebased this pull request as part of a merge.
  • Jun 13, 7:30 PM UTC: @ricokahler merged this pull request with Graphite.

@ricokahler ricokahler changed the base branch from v6_remove-array-revision-lock-check to graphite-base/38 June 13, 2025 19:25
@ricokahler ricokahler changed the base branch from graphite-base/38 to main June 13, 2025 19:27
…rnal APIs

BREAKING CHANGE: Remove `hideWarnings` option and associated warning system.
The diffPatch function no longer accepts or uses the hideWarnings option.

Remove console warnings when undefined values in arrays are converted to null.
This conversion behavior aligns with JSON.stringify() which also converts
undefined to null, making it relatively expected behavior that doesn't warrant
user warnings.

The console.log side effects made the library incompatible with some JavaScript
environments (like edge workers or serverless functions with restricted console
access) and the warning provided minimal value since the conversion behavior
matches standard JSON serialization.

Internal API changes:
- Remove options parameter threading through diffItem, diffObject, diffArray, etc.
- Simplify nullifyUndefined() to only handle the conversion without warnings
- Remove DEFAULT_OPTIONS constant
@ricokahler ricokahler force-pushed the v6_remove-undefined-to-null-warnings branch from e538df3 to 45ef4d1 Compare June 13, 2025 19:28
@ricokahler ricokahler merged commit 86cff6e into main Jun 13, 2025
7 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