Updating large and heavily nested JSON properties based on existing properties using nested forEach loops

Question

The structure of the JSON is as such:

Each parent record can have many DataGroups and each DataGroup can have many DataSets and each DataSet can have many DataFields.

What I've done is add a new property called columns on each DataSet. columns is a mapped array of simplified objects based on the DataFields of each DataSet.

I finally stringify the JSON and write to file.

My code works but I'm wondering if there is a better/more performant way of doing this.

    getMetadata()
  .then(async (response: any) => {

    const data = response?.data?.Data;

    data.forEach((parentRecord: any) => {
      parentRecord.DataGroups.forEach((datagroup: any) => {
        datagroup.DataSets.forEach(
          (dataset: { DataFields: any; columns?: any }) => {
            const fields = dataset.DataFields;
            const columns = fields?.map((x: any) => {
              return {
                Header:
                  x.UIPrettyName || x.OneReportPrettyName || x.FieldName,
                id: x.Id,
              };
            });
            dataset.columns = columns;
          }
        );
      });
    });

    appendFileSync(
      path.join(__dirname, "_metadata.json"),
      JSON.stringify(data),

Answer 1

I don't think there is a more 'performant' way of doing it, as you need to transform all elements. There might be slight tradeoffs over whether forEach is better/worse than for() (StackOverlow has posts on the topic) but I'd not be concerned by that unless you know the amount of data is large enough to justify trying to squeeze performance.

It's possible to reduce the verbosity on the code, although its arguable if that helps with readibility.

For clarity, I've extracted the handler function out.

function metadataHandler(response: any) {

    const data = response?.data?.Data;

    data?.forEach((parentRecord: any) => {
        parentRecord?.DataGroups?.forEach((datagroup: any) => {
            datagroup?.DataSets?.forEach(
                (dataset: any) => {
                    dataset.columns = dataset?.DataFields?.map((x: any) => ({
                        Header: x.UIPrettyName || x.OneReportPrettyName || x.FieldName,
                        id: x.Id,
                    }));
                }
            );
        });
    });
}

1. async is not necessary as the handler does nothing async.

2. const data = response?.data?.Data. Optional chaining can result in undefined being assigned to data, which will lead to an exception on data.forEach. data?.forEach... will prevent an exception if that's the case.

3. Some of the optional chaining can be removed if you are certain that the response is in fact the correct shape. Whilst any will allow property access we could easily introduce typos into the remainder of the loops and end up with the wrong result. If you know what your data looks like, it might be worth defining types to represent it (I'm guessing at the data types string and number here).

   type DataField = { UIPrettyName: string, OneReportPrettyName: string,  FieldName: string, Id: number }
   type DataSet = { DataFields: DataField[], columns?: { Header: string, id: number }
   type DataGroup = { DataSets: DataSet[] }
   type Data = { parentRecord: { DataGroups: DataGroup[] } }[]
   type MetadataResponse = { data: { Data: Data } }
   

4. We can then use a type guard to narrow the type of response

   function isMetadataResponse(obj: any): obj is MetadataResponse {
       if (typeof obj === 'object'
           && Array.isArray(obj.data?.Data)) {
           // let's assume it's enough, but we could add additional checks.
           return true
       }
       return false
   }
   

5. This gives us a cleaner handler implementation which provides type safety.

   function metadataHandler(response: any) {
   
       if (isMetadataResponse(response)) {
           const data = response.data.Data;
   
           data.forEach(item => {
               item.parentRecord.DataGroups.forEach(datagroup => {
                   datagroup.DataSets.forEach(dataset => {
                           dataset.columns = dataset.DataFields.map(x => ({
                               Header: x.UIPrettyName || x.OneReportPrettyName || x.FieldName,
                               id: x.Id,
                           }));
                       }
                   );
               });
           });
       }
   }
   

6. Use of forEach is modifying response by virtue of 'pass by reference'. This may be intentional, but could lead to unintended side effects if the response is handled later, for example because this handler is part of a middleware chain. You might want instead to convert this to use Array.map so that you are returning a transformed object graph.

7. Not sure if its intended to leave DataFields alongside the mapped columns as that seems redundant. Again, map might be the solutuion here.

Apologies if there are typos or errors in the above, not easy to test without some expected input and intended output.