search

firecmsco / firecms

Awesome Firebase/Firestore-based CMS. The missing admin panel for your Firebase project!
https://firecms.co
Other
1.13k stars 185 forks source link

fix[resolving]: spreadChildren fix for #552 Issue #554

Closed mvarchdev closed 11 months ago

mvarchdev commented 11 months ago

Added resolver checker for future, replaced collection with resolvedCollection for better optimisation

fixes #552

ghost commented 11 months ago
👇 Click on the image for a new way to code review #### [![Review these changes using an interactive CodeSee Map](https://s3.us-east-2.amazonaws.com/maps.codesee.io/images/github/firecmsco/firecms/554/eb54c404/5343a61c216e157272e4ad6934ad7a38a18d8016.svg)](https://app.codesee.io/r/reviews?pr=554&src=https%3A%2F%2Fgithub.com%2Ffirecmsco%2Ffirecms) #### Legend CodeSee Map legend
fgatti675 commented 11 months ago

We can't merge this.

This is a helper function which only purpose is to find a property or property builder in a tree. This should not resolve the property, specially with empty values. A function that is named get... should not have any side effects.

In order to solve the spreadChildren bug, we should look for the specific code. This change may fix it, but it can introduce a lot of other issues.

Thank you for your effort but I am closing this PR as it is not the right way to fix it. Feel free to submit a new one that is fixing the bug in the relevant code.

mvarchdev commented 11 months ago

@fgatti675 thanks for input! then I have another fix for that :) instead of collection variable, we can use resolvedCollection

mvarchdev commented 11 months ago

@fgatti675 new PR: #560