feat(modify): BREAKING CHANGE - Change the return result to include warnings

src/modify.js

@@ -1,6 +1,9 @@
 import get from 'lodash/get';
 import mergeWith from 'lodash/mergeWith';
 
+const WARNING_TYPES = {
+  FIELD_TO_CHANGE_NOT_FOUND: 'FIELD_TO_CHANGE_NOT_FOUND',
+};
 /**
  *
  * @param {*} path
@@ -25,15 +28,22 @@ function standardizeAttrs(attrs) {
 }
 
 function rewriteFields(schema, fieldsConfig) {
-  if (!fieldsConfig) return null;
+  if (!fieldsConfig) return { warnings: null };
+  const warnings = [];
 
   const fieldsToModify = Object.entries(fieldsConfig);
 
   fieldsToModify.forEach(([shortPath, mutation]) => {
     const fieldPath = shortToFullPath(shortPath);
 
     if (!get(schema.properties, fieldPath)) {
-      return; // Ignore field non existent in original schema.
+      // Do not override/edit a field that does not exist.
+      // That's the job of config.create() method.
+      warnings.push({
+        type: WARNING_TYPES.FIELD_TO_CHANGE_NOT_FOUND,
+        message: `Changing field "${shortPath}" was ignored because it does not exist.`,
+      });
+      return;
     }
 
     const fieldAttrs = get(schema.properties, fieldPath);
@@ -49,13 +59,17 @@ function rewriteFields(schema, fieldsConfig) {
     );
 
     if (fieldChanges.properties) {
-      rewriteFields(get(schema.properties, fieldPath), fieldChanges.properties);
+      const result = rewriteFields(get(schema.properties, fieldPath), fieldChanges.properties);
+      warnings.push(result.warnings);
     }
   });
+
+  return { warnings: warnings.flat() };
 }
 
 function rewriteAllFields(schema, configCallback, context) {
   if (!configCallback) return null;
+
   const parentName = context?.parent;
 
   Object.entries(schema.properties).forEach(([fieldName, fieldAttrs]) => {
@@ -80,10 +94,21 @@ function rewriteAllFields(schema, configCallback, context) {
 
 export function modify(originalSchema, config) {
   const schema = JSON.parse(JSON.stringify(originalSchema));
+  // All these functions mutate "schema" that's why we create a copy above
 
-  rewriteFields(schema, config.fields);
-
+  const resultRewrite = rewriteFields(schema, config.fields);
   rewriteAllFields(schema, config.allFields);
 
-  return schema;
+  if (!config.muteLogging) {
+    console.warn(
+      'json-schema-form modify(): We highly recommend you to handle/report the returned `warnings` as they highlight possible bugs in your modifications. To mute this log, pass `muteLogging: true` to the config.'
+    );
+  }
+
+  const warnings = [resultRewrite.warnings].flat().filter(Boolean);
+
+  return {
+    schema,
+    warnings,
+  };
 }

src/tests/modify.test.js

@@ -102,6 +102,36 @@ const schemaAddress = {
   },
 };
 
+beforeAll(() => {
+  jest.spyOn(console, 'warn').mockImplementation(() => {});
+});
+
+afterAll(() => {
+  console.warn.mockRestore();
+});
+
+describe('modify() - warnings', () => {
+  it('logs a warning by default', () => {
+    const result = modify(schemaPet, {});
+
+    expect(console.warn).toBeCalledWith(
+      'json-schema-form modify(): We highly recommend you to handle/report the returned `warnings` as they highlight possible bugs in your modifications. To mute this log, pass `muteLogging: true` to the config.'
+    );
+
+    console.warn.mockClear();
+    expect(result.warnings).toEqual([]);
+  });
+
+  it('given muteLogging, it does not log the warning', () => {
+    const result = modify(schemaPet, {
+      muteLogging: true,
+    });
+
+    expect(console.warn).not.toBeCalled();
+    expect(result.warnings).toEqual([]);
+  });
+});
+
 describe('modify() - basic mutations', () => {
   it('replace base field', () => {
     const result = modify(schemaPet, {
@@ -119,7 +149,7 @@ describe('modify() - basic mutations', () => {
       },
     });
 
-    expect(result).toMatchObject({
+    expect(result.schema).toMatchObject({
       properties: {
         pet_name: {
           title: 'Your pet name',
@@ -135,32 +165,39 @@ describe('modify() - basic mutations', () => {
   it('replace nested field', () => {
     const result = modify(schemaAddress, {
       fields: {
+        // You can use dot notation
         'address.street': {
           title: 'Street name',
         },
         'address.city': () => ({
           title: 'City name',
         }),
-        address: () => {
+        // Or pass the native object
+        address: (fieldAttrs) => {
           return {
             properties: {
-              number: { title: 'Door number' },
+              number: (nestedAttrs) => {
+                return {
+                  'x-test-siblings': Object.keys(fieldAttrs.properties),
+                  title: `Door ${nestedAttrs.title}`,
+                };
+              },
             },
           };
         },
-        // TODO: write test to nested field
       },
     });
 
-    expect(result).toMatchObject({
+    expect(result.schema).toMatchObject({
       properties: {
         address: {
           properties: {
             street: {
               title: 'Street name',
             },
             number: {
-              title: 'Door number',
+              title: 'Door Number',
+              'x-test-siblings': ['street', 'number', 'city'],
             },
             city: {
               title: 'City name',
@@ -171,6 +208,44 @@ describe('modify() - basic mutations', () => {
     });
   });
 
+  it('replace fields that dont exist gets ignored', () => {
+    // IMPORTANT NOTE on this behavior:
+    // Context: At Remote we have a lot of global customization that run equally across multiple different JSON Schemas.
+    // With this, we avoid applying customizations to non-existing fields. (aka create fields)
+    // That's why we have the "create" config, specific to create new fields.
+    const result = modify(schemaPet, {
+      fields: {
+        unknown_field: {
+          title: 'This field does not exist in the original schema',
+        },
+        'nested.field': {
+          title: 'Nop',
+        },
+        pet_name: {
+          title: 'New pet title',
+        },
+      },
+    });
+
+    expect(result.schema.properties.unknown_field).toBeUndefined();
+    expect(result.schema.properties.nested).toBeUndefined();
+    expect(result.schema.properties.pet_name).toEqual({
+      ...schemaPet.properties.pet_name,
+      title: 'New pet title',
+    });
+
+    expect(result.warnings).toEqual([
+      {
+        type: 'FIELD_TO_CHANGE_NOT_FOUND',
+        message: 'Changing field "unknown_field" was ignored because it does not exist.',
+      },
+      {
+        type: 'FIELD_TO_CHANGE_NOT_FOUND',
+        message: 'Changing field "nested.field" was ignored because it does not exist.',
+      },
+    ]);
+  });
+
   it('replace all fields', () => {
     const result = modify(schemaPet, {
       allFields: (fieldName, fieldAttrs) => {
@@ -188,7 +263,7 @@ describe('modify() - basic mutations', () => {
       },
     });
 
-    expect(result).toMatchObject({
+    expect(result.schema).toMatchObject({
       properties: {
         has_pet: {
           dataFoo: 'abc',
@@ -203,6 +278,7 @@ describe('modify() - basic mutations', () => {
           styleDecimals: 2,
         },
         pet_address: {
+          // assert recursivity
           properties: {
             street: {
               dataFoo: 'abc',
@@ -239,7 +315,7 @@ describe('modify() - basic mutations', () => {
       },
     });
 
-    expect(result).toMatchObject({
+    expect(result.schema).toMatchObject({
       properties: {
         has_pet: {
           oneOf: [
@@ -266,7 +342,7 @@ describe('modify() - basic mutations', () => {
       },
     });
 
-    expect(result).toMatchObject({
+    expect(result.schema).toMatchObject({
       properties: {
         has_pet: {
           oneOf: [
@@ -300,7 +376,7 @@ describe('modify() - basic mutations', () => {
       // ...
     });
 
-    expect(result).toMatchObject({
+    expect(result.schema).toMatchObject({
       properties: {
         pet_age: {
           'x-jsf-errorMessage': {