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

Should stamp/core/mergeOne() handle undefined as its first parameter? #55

Open
PopGoesTheWza opened this issue Nov 4, 2019 · 4 comments

Comments

@PopGoesTheWza
Copy link
Contributor

PopGoesTheWza commented Nov 4, 2019

Current implementation of @stamp/core/mergeOne() does not fully check if its first parameter dst is undefined. If the second parameter src is an object, dst is never checked and assumed to be an array/object and thus a TypeError: Cannot read property 'foo' of undefined is thrown.

  • stamp/packages/collision/index.js line 33

This issue is triggered by unit tests and standardiseDescriptor() (in packages/it/index.js lines:41, 47 and 56

This issue relates with #52

@koresar
Copy link
Member

koresar commented Nov 4, 2019

Yeah, you're right. It should handle the first undefined argument.

@PopGoesTheWza
Copy link
Contributor Author

I can PR this (provided I get branching rights)

@koresar
Copy link
Member

koresar commented Nov 4, 2019

I think I have provided all the member the Write access to all stampit-org repos few minutes ago. Could you please check if I did it right?

@PopGoesTheWza
Copy link
Contributor Author

It looks ok now. Thanks.

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

2 participants