Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Addition of style on createAnimateElement arg breaks our propTypes validation #92

Open
majapw opened this issue Feb 14, 2018 · 6 comments

Comments

@majapw
Copy link

majapw commented Feb 14, 2018

Hi there!

I just started seeing test failures as a result of

style={ApplyAnimatedValues.transformStyles(style)}
which was added in v0.2.2.

Basically, our tests validate prop types (which we always forbid extraneous prop types) and we sometimes pass actual components to Animated.createAnimatedComponents (which have their own prop types definitions). So when style started getting applied to the arg passed in... our tests failed.

Is there any way to maybe export a prop type that can be used for these components or any sort of guidance on this behavior (because we're passing in a component, I don't think the style attribute will even work as expected). Should we not be passing in a component?

Thanks!

FYI @ljharb @backwardok

@browniefed
Copy link
Member

Figured I'd break something for someone. The issue was that transform properties were not being applied on initial mount, so switched to needing to process style.transform on re-render.

The PropType for it should just be an object that isn't required, and or undefined if no style prop is passed in. However this has a dual target (one for web/one for native) so it could be an object, number, array or undefined.

Does your system look for the presence of a prop even in 3rd party libraries like this one? Since it would only end up passing down undefined which shouldn't even register as a prop being passed?

@majapw
Copy link
Author

majapw commented Feb 15, 2018 via email

@ljharb
Copy link

ljharb commented Feb 15, 2018

undefined always registers as the prop being passed, and it’s up to the validator to ignore it. It’s property presence that triggers the validation.

@ljharb
Copy link

ljharb commented Feb 15, 2018

The purpose of https://npmjs.com/prop-types-exact is to avoid bugs; and unintentionally passing undefined on an invalid prop name is a common bug, especially with third party libs. Loosening the exact checker for undefined wouldn’t help us.

Note that on html elements, React will log a runtime error if you pass some attributes as undefined rather than omitting them. Would it be possible to omit the prop entirely when it’s not needed?

@browniefed
Copy link
Member

Will this fix your system?
https://github.com/animatedjs/animated/pull/93/files

@ljharb
Copy link

ljharb commented Feb 15, 2018

(actually linking to #93)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants