Fix duplicate attributes for kebab-case props (#196)

Author: Nir Maoz

src/Util/Util.js

@@ -1,2 +1,2 @@
-export extractCloudinaryProps from './extractCloudinaryProps';
 export * from './cloudinaryCoreUtils';
+export * from './cloudinaryReactUtils'

src/Util/extractCloudinaryProps.js src/Util/cloudinaryReactUtils.jssrc/Util/extractCloudinaryProps.js renamed to src/Util/cloudinaryReactUtils.js

@@ -1,27 +1,24 @@
-import {Transformation, Util} from 'cloudinary-core';
-
-const CLOUDINARY_REACT_PROPS = {includeOwnBody: true};
+import {Transformation, Util} from "cloudinary-core";
+// props passed to cloudinary-core but should not be rendered as dom attributes
+const CLOUDINARY_REACT_PROPS = ['accessibility', 'breakpoints', 'dataSrc', 'placeholder', 'publicId', 'signature'];
 
+// props passed to cloudinary-core for dom elements attributes generation
 // Map Cloudinary props from array to object for efficient lookup
-const CLOUDINARY_PROPS = Transformation.PARAM_NAMES.map(Util.camelCase).concat(['publicId', 'breakpoints']).reduce(
+const CLOUDINARY_PROPS = [...Transformation.PARAM_NAMES, ...CLOUDINARY_REACT_PROPS].map(Util.camelCase).reduce(
   (accumulator, cloudinaryPropName) => {
     accumulator[cloudinaryPropName] = true;
     return accumulator;
   },
   {}
 );
 
-const isDefined = (props, key) => {
-  return (props[key] !== undefined && props[key] !== null);
-};
-
 /**
  * Extracts cloudinaryProps and nonCloudinaryProps from given props
  *
  * @param props
  * @returns {{children: *, cloudinaryReactProps: {}, cloudinaryProps: {}, nonCloudinaryProps: {}}}
  */
-export default ({children, ...props}) => {
+const extractCloudinaryProps = ({children, ...props}) => {
   let result = {
     children,
     cloudinaryProps: {},
@@ -35,10 +32,10 @@ export default ({children, ...props}) => {
 
     //if valid and defined add to cloudinaryProps
     if (CLOUDINARY_PROPS[camelKey]) {
-      if (isDefined(props, key)) {
+      if (props[key] !== undefined && props[key] !== null) {
         result.cloudinaryProps[camelKey] = value;
       }
-    } else if (CLOUDINARY_REACT_PROPS[camelKey]) { //cloudinary-react spesific prop
+    } else if (camelKey === 'includeOwnBody') { //cloudinary-react specific prop
       result.cloudinaryReactProps[camelKey] = value;
     } else { //not valid so add to nonCloudinaryProps
       result.nonCloudinaryProps[key] = value;
@@ -47,3 +44,8 @@ export default ({children, ...props}) => {
 
   return result;
 };
+
+export {
+  CLOUDINARY_REACT_PROPS,
+  extractCloudinaryProps
+};

src/components/Image/Image.js

@@ -1,6 +1,6 @@
 import React, {createRef, Fragment} from 'react';
 import CloudinaryComponent from '../CloudinaryComponent';
-import {extractCloudinaryProps, getImageTag, makeElementResponsive} from "../../Util";
+import {CLOUDINARY_REACT_PROPS, extractCloudinaryProps, getImageTag, makeElementResponsive} from "../../Util";
 import {Util} from "cloudinary-core";
 import PropTypes from "prop-types";
 
@@ -56,11 +56,13 @@ class Image extends CloudinaryComponent {
    */
   getAttributes = (additionalOptions = {}) => {
     const {placeholder} = additionalOptions;
-    const options = {...this.getOptions(), ...additionalOptions};
-    const {nonCloudinaryProps} = extractCloudinaryProps(options);
+    const options = extractCloudinaryProps({...this.getOptions(), ...additionalOptions});
+    const {cloudinaryProps, nonCloudinaryProps, cloudinaryReactProps} = options;
+    const imageTag = getImageTag({...cloudinaryProps, ...cloudinaryReactProps});
+    const cloudinaryAttributes = Util.withCamelCaseKeys(imageTag.attributes());
 
     // React requires camelCase instead of snake_case attributes
-    const attributes = ({...Util.withCamelCaseKeys(getImageTag(options).attributes()), ...nonCloudinaryProps});
+    const attributes = ({...cloudinaryAttributes, ...nonCloudinaryProps});
 
     // We want to keep 'data-src' if it exists
     if (attributes.dataSrc) {
@@ -79,7 +81,7 @@ class Image extends CloudinaryComponent {
     }
 
     // Remove unneeded attributes,
-    ['dataSrc', 'accessibility', 'placeholder', 'breakpoints'].forEach(attr => {
+    CLOUDINARY_REACT_PROPS.forEach(attr => {
       delete attributes[attr];
     });
 

src/components/Video/Video.js

@@ -2,7 +2,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import {Cloudinary, Util} from 'cloudinary-core';
 import CloudinaryComponent from '../CloudinaryComponent';
-import extractCloudinaryProps from "../../Util/extractCloudinaryProps";
+import {extractCloudinaryProps} from "../../Util";
 
 /**
  * A component representing a Cloudinary served video

test/ImageTest.js

@@ -94,13 +94,19 @@ describe('Image', () => {
 
     expect(tag.find('img').prop('src')).to.match(/fn_wasm:blur.wasm\/sample/);
   });
-  it('should not change kebab-case param names', () => {
+  describe('prop names', () => {
     let tag = mount(
-      <Image publicId="sample" cloudName="demo" data-testid="testing"/>
+      <Image publicId="sample" cloudName="demo" data-testid="testing" aria-describedby="testValue"/>
     );
-
-    expect(tag.find('img').prop('data-testid')).to.equal('testing');
-    expect(tag.find('img').prop('datatestid')).to.equal(undefined);
+    it('should not change kebab-case prop names', () => {
+      expect(tag.find('img').prop('data-testid')).to.equal('testing');
+      expect(tag.find('img').prop('aria-describedby')).to.equal('testValue');
+    });
+    it('should not duplicate attributes for kebab-case props', () => {
+      expect(tag.find('img').prop('datatestid')).to.not.exist;
+      expect(tag.find('img').prop('ariadescribedby')).to.not.exist;
+      expect(tag.html()).to.equal('<img src="http://res.cloudinary.com/demo/image/upload/sample" data-testid="testing" aria-describedby="testValue">');
+    });
   });
   it('should update on prop change', () => {
     let tag = mount(<Image publicId="sample" cloudName="demo"/>);