Skip to content

Resolve PR #2585 Change Requests - #2609

Merged
lucas-koehler merged 3 commits into
eclipsesource:fix-2397from
darius-lesch:resolve/pr-2585-change-requests
Jul 31, 2026
Merged

Resolve PR #2585 Change Requests#2609
lucas-koehler merged 3 commits into
eclipsesource:fix-2397from
darius-lesch:resolve/pr-2585-change-requests

Conversation

@darius-lesch

@darius-lesch darius-lesch commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

The PR resolves two comments in #2585:

1. packages/core/src/util/setData.ts

The use of the loose != null was originally intentional, as it is a standard JavaScript shorthand to check for both null and undefined simultaneously. We can't use !data in this context because it would be unsafe; it would incorrectly reject valid falsy data values that we want to keep, such as 0, false, or an empty string "".

However, I completely agree that loose inequality can look like a typo or cause confusion at first glance. To make the intent strictly clear and avoid any "weirdness", I've updated the logic to use the explicit multiple cases as you suggested: data !== null && data !== undefined.

2. MIGRATION.md

I have also moved the data update path documentation into a new ## Migrating to JSON Forms 3.9 section in MIGRATION.md.

Added heading "Migration to JSON Forms 3.9" and moved "Data update paths
treat all segments literally" from "Mifration to JSON Forms 3.8" to
"Migration to JSON FOrms 3.9". This resolves the change request comment
for `MIGRATION.md` file in PR eclipsesource#2585.
Added strict equality checks (`!== null && !== undefined`) inside
`ownPropertyValue`. This resolves comment in PR eclipsesource#2585 regarding use of loose inequality.
@darius-lesch

Copy link
Copy Markdown
Contributor Author

@sdirix : Hope this helps resolve the change requests in PR #2585, get milestone 3.9 complete. and Let me know if there's anything else needed to get v3.9 shipped!

@sdirix

sdirix commented Jul 31, 2026

Copy link
Copy Markdown
Member

Hi @darius-lesch, do you need JSON Forms 3.9 urgently shipped for your use cases? Would a pre-release of the current state be sufficient?

@darius-lesch

Copy link
Copy Markdown
Contributor Author

Hi @darius-lesch, do you need JSON Forms 3.9 urgently shipped for your use cases? Would a pre-release of the current state be sufficient?

Hi @sdirix , no, there is no immediate urgency for my use cases. A pre-release of the current state is more than sufficient.

@lucas-koehler lucas-koehler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @darius-lesch , thanks for the fixes ❤️
There is a small lint error due to expected formatting in the changed code line.

@darius-lesch

Copy link
Copy Markdown
Contributor Author

Hi @darius-lesch , thanks for the fixes ❤️ There is a small lint error due to expected formatting in the changed code line.

Hi @lucas-koehler , linting issue has been resolved now.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 84.286%. remained the same — darius-lesch:resolve/pr-2585-change-requests into eclipsesource:fix-2397

@lucas-koehler lucas-koehler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM now, thanks for the update!

@lucas-koehler
lucas-koehler merged commit 1616bf2 into eclipsesource:fix-2397 Jul 31, 2026
6 of 7 checks passed
@lucas-koehler

Copy link
Copy Markdown
Contributor

@darius-lesch a pre-release 3.9.0-alpha.0 should now be available containing all latest commits :)

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.

4 participants