VulcanJS / vulcan-npm

The full-stack JavaScript App Framework
https://vulcan-docs.vercel.app
MIT License
30 stars 8 forks source link

Fix useUpdate hook and improve defaultMutations and mutators dependency #62

Closed Timi-Duban closed 3 years ago

Timi-Duban commented 3 years ago

Two big things here:

This is a draft PR because I need to think about all the TODO on these documents, and to fix and improve the tests that are currently broken because of the changes.

About #33 and maybe #59 it needs to be tested Closes #33

Timi-Duban commented 3 years ago

Just did a todo: data validation functions where splitted in 2 (validateDocument and validateData) that were called by validateMutationData in a tricky way in the mutator. Now I think it's clearer and safer, tests are updated too and they're all good.

eric-burel commented 3 years ago

Just did a todo: data validation functions where splitted in 2 (validateDocument and validateData) that were called by validateMutationData in a tricky way in the mutator. Now I think it's clearer and safer, tests are updated too and they're all good.

Great work so far!! Yes indeed this function was dirty. Just add a comment at the top of the file explaning this change: some old applications may use those functions in their own code. I am not sure, so at least add a comment saying smth like "Differences with vulcan Meteor: - removed validateDocument and refactored validateData, so now we use only validateData" so we can remember this modification compared to Vulcan Meteor.

eric-burel commented 3 years ago

@Timi-Duban I think we're good, the last comments are pure form but it seems that you have covered everything at this point. Does it work ok when you plug Vulcan Next? If yes I'll merge that

Thanks for this huge refactor!

eric-burel commented 3 years ago

:1st_place_medal: