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

Adds support for copying static symbol properties. #17

Merged
merged 2 commits into from
Jun 17, 2016

Conversation

Tarabyte
Copy link
Contributor

Support copying symbol properties to make this function more Object.assign like.

Native Object.assign copies both string and symbol properties. MDN.

This might be useful when wrapped component was annotated using Symbol to exclude name collision.

@coveralls
Copy link

coveralls commented Jun 17, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 59d82c7 on Tarabyte:symbols into c94d6c0 on mridgway:master.

@mridgway
Copy link
Owner

I'm OK with this, but for browser compatibility reasons I think we should ensure that Object. getOwnPropertySymbols exists before calling it.

@Tarabyte
Copy link
Contributor Author

Do you think simple typeofcheck would be enough?

javascript
var isGetOwnPropertySymbolsAvailable = typeof Object.getOwnPropertySymbols === 'function';

module.exports = function hoistNonReactStatics(targetComponent, sourceComponent, customStatics) {
if (typeof sourceComponent !== 'string') { // don't hoist over string (html) components
var keys = Object.getOwnPropertyNames(sourceComponent);

    if (isGetOwnPropertySymbolsAvailable) {
        keys = keys.concat(Object.getOwnPropertySymbols(sourceComponent));
    }

@mridgway
Copy link
Owner

Yeah, that looks fine to me.

@coveralls
Copy link

coveralls commented Jun 17, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 1d86a81 on Tarabyte:symbols into c94d6c0 on mridgway:master.

@mridgway
Copy link
Owner

👍

@mridgway mridgway merged commit 49c4252 into mridgway:master Jun 17, 2016
@mridgway
Copy link
Owner

mridgway commented Jun 17, 2016

Thanks @Tarabyte! This was published in v1.2.0.

@Tarabyte
Copy link
Contributor Author

Wow, that was fast! Thank you! 👍

@Tarabyte Tarabyte deleted the symbols branch June 17, 2016 19:02
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

Successfully merging this pull request may close these issues.

None yet

3 participants