schemaForm.merge regression in 1.0.0-alpha.5

[donalmurtagh]

I recently upgraded from 0.8.13 to the latest 1.0.0-alpha.5. In my app, I programmatically merge the form and schema definitions by calling the following function, which serves as a simple facade for
schemaForm.merge

    function mergeFormAndSchema(form, schema) {
      return schemaForm.merge(schema, form, {}, {}, undefined, []);
    }

I have some Jasmine tests of this function, which fail since upgrading from 0.8.13 to the latest alpha

Test 1

  it('should merge form and schema', function () {

    var form = [
      {
        key: 'foo',
        title: 'bar',
        type: 'xyz'
      },
      {
        key: 'noschema'
      }
    ];

    var schema = {
      type: 'object',
      properties: {
        foo: {
          type: 'string'
        }
      },
      required: ['foo']
    };

    var result = schemaFormHelperService.mergeFormAndSchema(form, schema);

    expect(result[0].key).toEqual(['foo']);
    expect(result[0].required).toEqual(true);  // FAIL
    expect(result[0].type).toEqual('xyz');
    expect(result[0].schema.type).toEqual('string'); // FAIL

    expect(result[1].key).toEqual(['noschema']);
  });

I've marked the lines that fail with // FAIL but ultimately the reason for the failure is simply because each object in result has less data in the latest alpha than in 0.8.13

result in 0.8.13

[
  {
    "key": [
      "foo"
    ],
    "title": "bar",
    "type": "xyz",
    "required": true,
    "schema": {
      "type": "string"
    },
    "ngModelOptions": {}
  },
  {
    "key": [
      "noschema"
    ]
  }
]

result in 1.0.0-alpha.5

[
  {
    "key": [
      "foo"
    ],
    "title": "bar",
    "type": "xyz"
  },
  {
    "key": [
      "noschema"
    ]
  }
] 

Test 2

  it('should traverse basic form', function () {

    var form = ['*'];
    var schema = {
      type: 'object',
      properties: {
        foo: {
          type: 'string'
        },
        bar: {
          type: 'object',
          properties: {
            child: {
              type: 'number'
            }
          }
        }
      },
      required: ['foo']
    };
    var mergedForm = schemaFormHelperService.mergeFormAndSchema(form, schema);
    expect(mergedForm.length).toEqual(2); // FAIL
  });

In the latest 1.0.0-alpha.5, mergedForm is an empty list, whereas in 0.8.13 it is

[
  {
    "title": "foo",
    "required": true,
    "schema": {
      "type": "string"
    },
    "ngModelOptions": {},
    "key": [
      "foo"
    ],
    "type": "text"
  },
  {
    "title": "bar",
    "schema": {
      "type": "object",
      "properties": {
        "child": {
          "type": "number"
        }
      }
    },
    "ngModelOptions": {},
    "type": "fieldset",
    "items": [
      {
        "title": "child",
        "schema": {
          "type": "number"
        },
        "ngModelOptions": {},
        "key": [
          "bar",
          "child"
        ],
        "type": "number"
      }
    ]
  }
]

Is it the case that

• this is a bug in 1.0.0-alpha.5
• the contract (arguments) of schemaForm.merge has changed in 1.0.0-alpha.5, and I need to call it differently in order to get the same results as in version 0.8.3
• schemaForm.merge was not intended to be used outside of ASF (and therefore no attempt is made to maintain backwards compatability)

@json-schema-form/angular-schema-form-lead

[Anthropic]

Thanks @donalmurtagh the arguments have changed on this one, the function was moved to the core library and therefore needed more information.

You can see the different signature here: schema-form.provider.js#L103

[donalmurtagh]

Thanks @Anthropic, I replaced

    function mergeFormAndSchema(form, schema) {
      return schemaForm.merge(schema, form, {}, {}, undefined, []);
    }

with

    function mergeFormAndSchema(form, schema) {
      return schemaForm.merge(schema, form);
    }

and my tests pass now. If you're planning on writing an upgrade/migration guide for ASF v1.0, the change in this function's signature is worth mentioning.

[Anthropic]

@donalmurtagh Yeah I added the "needs documentation" tag and I will review everything with that tag regardless of it being open or closed when I am ready to go to beta, thanks again, can't tell you how helpful it is to have someone going through an upgrade process to help me find things I may have forgotten or missed :)

[donalmurtagh]

@Anthropic you're very welcome and your timely responses are much appreciated

[Anthropic]

@donalmurtagh I added #894 to make sure I keep track of items to document, please let me know what you think may be of use that isn't a breaking change, it is good the way you make an issue for each so I can confirm it is an actual change vs a bug. I'll close this issue now as once the documentation is done it will be covered.