KrysKruk / serverless-import-config-plugin

Split your serverless.yaml config file into smaller modules and import them.
MIT License
10 stars 6 forks source link

When merging configurations array elements comparison function is shallow #8

Closed kgenge closed 4 years ago

kgenge commented 4 years ago

Hi there ! Thank you for supporting the plugin it's really great. Unfortunately currently I have an issue with merging configuration fragments that contain elements in array which are objects. Take the 2 logically identical fragments for merging:

- DomainName:
    'Fn::Join':
        - ''
        - - Ref: componentsBucket
           - '.s3.${opt:region,self:provider.region}.amazonaws.com'
- DomainName:
    'Fn::Join':
        - ''
        - - Ref: componentsBucket
           - '.s3.${opt:region,self:provider.region}.amazonaws.com'

Due to the fact that uniqueness of the array elements is shallow the {Ref: componentsBucket} element will be added to the merged array which is not what we expect. current code in merge.ts:10 :

if (Array.isArray(target) && Array.isArray(source)) {
    // add unique elements of source into target
    return target.concat(source.filter(elem => !target.includes(elem)))
  }

To make it work not only for string elements you could consider using lodash differenceWith and isEqual functions to perform a deep array elements comparison. proposed solution:

if (Array.isArray(target) && Array.isArray(source)) {
    // add unique elements of source into target with deep elements equality comparison
    return target.concat(differenceWith(source, target, isEqual));
  }

If you agree with the proposed solution, could you kindly accept the PR and publish a new version to npm repository?

KrysKruk commented 4 years ago

Sorry for the late answer. The PR has been merged. I will publish new version right now. Thank you very much for this change!