wpengine / faustjs

Faust.js™ - The Headless WordPress Framework
https://faustjs.org
Other
1.43k stars 132 forks source link

Should flatListToHierarchical default params correspond to current content block props #1506

Open JEverhart383 opened 1 year ago

JEverhart383 commented 1 year ago

When implementing a new project using Faust and WPGraphQL content blocks, I stumbled when converting my list of blocks into a tree because the default client and parent id keys for the function are different from what content blocks return.

We should consider changing the default params to correspond to what GraphQL will return so that no additional work is needed beyond passing in the array of blocks.

In addition, this doc should be updated to reflect the updated properties that WPGraphQL returns related to ids and show the need to pass an additional param to this utility function:

const blocks = flatListToHierarchical(editorBlocks, {idKey: 'clientId', parentKey: 'clientParentId'});

It looks like the docs also include an example where clientId is aliased as id, which is maybe another way to approach this in examples, but I'm not sure we can assume that everyone coming to Faust will know what that means and how to do it in GraphQL parlance. If we do go that route, we should call out why we're doing this and link to the GraphQL docs on aliasing.

theodesp commented 1 year ago

Hey @JEverhart383 Changing the defaults could mean a breaking change since it would break existing clients using the flatListToHierarchical. Maybe we should document the flatListToHierarchical better instead of bumping a major version.

theodesp commented 1 year ago

Also flatListToHierarchical is used in many places and not only when handling blocks. For example when rendering menus and such.

JEverhart383 commented 1 year ago

You are absolutely right @theodesp about the breaking change. I thought about that right after I posted this comment. And I wasn't aware that it had other uses, so that is another reason not to change the API. Forget my suggestion there. I think either showing how to call the function with override ids or recommending aliases in the query are both good options, and maybe showing people both and letting them decide.