Skip to content

Conversation

@susnux
Copy link
Contributor

@susnux susnux commented Dec 22, 2025

We cannot use instanceof because two apps will have different libraries so the prototype is not the same and the check will always fail.
Instead validate the column using the already existing validation method.

@susnux susnux added type: bug 🐛 Something isn't working 3. to review 3️⃣ Waiting for reviews labels Dec 22, 2025
@susnux susnux added this to the v4.0.0 milestone Dec 22, 2025
@codecov
Copy link

codecov bot commented Dec 22, 2025

Codecov Report

❌ Patch coverage is 58.82353% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.64%. Comparing base (eaac8df) to head (6a3a582).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
lib/navigation/column.ts 40.00% 3 Missing ⚠️
lib/navigation/view.ts 50.00% 1 Missing and 1 partial ⚠️
lib/utils/objectValidation.ts 75.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1442      +/-   ##
==========================================
+ Coverage   89.23%   89.64%   +0.41%     
==========================================
  Files          25       26       +1     
  Lines         706      705       -1     
  Branches      190      189       -1     
==========================================
+ Hits          630      632       +2     
+ Misses         63       59       -4     
- Partials       13       14       +1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

We cannot use `instanceof` because two apps will have different
libraries so the prototype is not the same and the check will always
fail.
Instead validate the column using the already existing validation method.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Copy link

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

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

Fixes the issue, but I'm not very fond of the solution..

IMO, validation properties is neither reliable (no guarantee it actually checks the type) nor direct way to check the type (we check properties, not type).

I'd either expect using classes and then check the constructor name (like instancof, but a string, so no cross-frontend issues). Or allowing using data objects and then adding a flag property like __view__ so we can only check a single private property added by the instance builder/creator.

And if the goal is to replace TS with JS in runtime, I'd consider using one of some good libraries to transform type declaration into validation, to not write the same checks over and over...

@susnux
Copy link
Contributor Author

susnux commented Dec 23, 2025

Agree. I think for the pure data objects like view, filter or column we should have a better way.
But the previous class approach was very inflexible because it was not possible to extend the classes to add state.

@susnux susnux merged commit 5279304 into main Dec 23, 2025
11 of 12 checks passed
@susnux susnux deleted the fix/column branch December 23, 2025 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review 3️⃣ Waiting for reviews type: bug 🐛 Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants