Skip to content

Override isCollection for custom objects #203

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
markdascher opened this issue Jun 4, 2025 · 4 comments · Fixed by #204
Closed

Override isCollection for custom objects #203

markdascher opened this issue Jun 4, 2025 · 4 comments · Fixed by #204
Assignees
Labels
should do user request Requested by user

Comments

@markdascher
Copy link

Suggestion
Provide some way to use custom objects without treating them all as collections. There's a showCollectionWrapper property which sounds close, except it just hides the braces. It doesn't get rid of the extra line break, expand/contract functionality, etc. Maybe something like isCollection: false in customNodeDefinitions.

In our case, only "plain" Objects are collections, since we're parsing JSON (override some specific nodes with a custom object) and then passing in the result. So some logic like value.constructor === Object might also work. That probably wouldn't work in general, though.

Use case
We'd like to parse JSON using the lossless-json library, which puts numbers into a LosslessNumber type. The purpose is to show formatted JSON that doesn't modify the input at all. It doesn't translate 1.0000 into 1 or translate 123456789012345678901234567890 into 1.2345678901234568e+29, etc.

We can almost make this work with json-edit-react already, but it treats LosslessNumber as a collection.

@markdascher markdascher added the user request Requested by user label Jun 4, 2025
@CarlosNZ
Copy link
Owner

CarlosNZ commented Jun 4, 2025

Hey, thanks for the suggestion. I actually had to deal with this issue myself when I made a custom handler for Date objects: https://github.com/CarlosNZ/json-edit-react/tree/main/custom-component-library/components/DateObject

And yeah, I had the exact same issue -- the Date object was treated as a collection. I actually put a sneaky exception for "Date" objects into the isCollection check:

export const isCollection = (value: unknown): value is Record<string, unknown> | unknown[] =>
value !== null && typeof value === 'object' && !(value instanceof Date)

But yeah, that's pretty specific for Dates, and as you've shown, there are other object types that we might want to not display as collections. I like your suggestion of an addition isCollection prop for the custom definitions. I'll try and add that in in the next week or so.

@CarlosNZ CarlosNZ self-assigned this Jun 4, 2025
@markdascher
Copy link
Author

That would be fantastic, thanks! And there's no rush of course–if it takes longer than a week that's perfectly fine too. 😄 We've come up with a very silly workaround (which I won't admit publicly) in the meantime…

CarlosNZ added a commit that referenced this issue Jun 7, 2025
* Evaluate custom nodes in parent, create "Enhanced Link" component
* Update README.md
@CarlosNZ CarlosNZ reopened this Jun 7, 2025
@CarlosNZ
Copy link
Owner

CarlosNZ commented Jun 7, 2025

Okay, I've done this but not published a release yet, will release later this weekend.

We've come up with a very silly workaround (which I won't admit publicly) in the meantime

Ha, some ugly CSS hackery I'm guessing?

@CarlosNZ
Copy link
Owner

CarlosNZ commented Jun 8, 2025

Hi @markdascher, this is done now and available in v1.27.0

I've called the prop renderCollectionAsValue, and it's documented here.

There are a couple of example implementations:

Hope that works well for you. If you think your custom "lossless number" component would be useful, you might want to contribute it to the Custom Component library :)

@CarlosNZ CarlosNZ closed this as completed Jun 8, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
should do user request Requested by user
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants