adriantoine / enzyme-to-json

Snapshot test your Enzyme wrappers
MIT License
947 stars 64 forks source link

Not consistent behaviour when removing default props #99

Closed tkomstadius closed 5 years ago

tkomstadius commented 6 years ago

This was discovered in 3.3.2 that is no longer out. (See comments on https://github.com/adriantoine/enzyme-to-json/commit/f5181d448c45c8dfaaea44b85100b7a1e6be1516)

When the snapshots got created the default props containing Object {}, [Function] and boolean values seems to be removed but not the empty strings.

In our component Image we have default props:

Image.defaultProps = {
    className: '',
    caption: '',
    style: {},
};

Snapshot for version 3.3.1:

<Image
    caption=""
    className=""
    height={60}
    style={Object {}}
    width={60}
/>

Snapshot for version 3.3.2

<Image
    caption=""
    className=""
    height={60}
    width={60}
/>
adriantoine commented 6 years ago

Hey @tkomstadius, thanks for opening the bug, that feature will be refactored anyway as I want it as an opt-in feature.

andybarron commented 6 years ago

The current implementation removes props if their values equal defaults, right? What if we temporarily un-assigned Component.defaultProps before calling React.createElement or something instead? Mega hack, but the output might be cleaner?

phillipwhelan commented 6 years ago

Is there a timeline for when this fix will be released?

sattaman commented 6 years ago

would be ace to have this released !

VincentLanglet commented 5 years ago

@tkomstadius I think it's because '' is a falsy value. This PR will fix this. https://github.com/adriantoine/enzyme-to-json/pull/112/files

@adriantoine Hi, do you have time for this opt-in , do you need help ?

I think

function getProps(node, options) {
  const props = omitBy(
    Object.assign({}, propsOfNode(node)),
    (val, key) => {
      if (key === 'children' || val === undefined) {
        return true;
      }

      if (
        options.ignoreDefaultProps === true
        typeof node.type === 'function' &&
        node.type.defaultProps &&
        key in node.type.defaultProps &&
        node.type.defaultProps[key] === val
      ) {
        return true;
      }
    }
  );

  if (!isNil(node.key) && options.noKey !== true) {
    props.key = node.key;
  }

  return props;
}

Will do the job and can be usable like this

import {createSerializer} from 'enzyme-to-json';
expect.addSnapshotSerializer(createSerializer({ ignoreDefaultProps: true }));
VincentLanglet commented 5 years ago

The opt-in feature is released