Skip to content
This repository has been archived by the owner on Aug 24, 2021. It is now read-only.

Fix incompatibility with zii via @rvagg's fix #84

Merged
merged 3 commits into from
Feb 26, 2021

Conversation

KyleMaas
Copy link
Contributor

See #80 (comment)

Tested with and without zii, and works in both cases.

Fixes #80.

@KyleMaas
Copy link
Contributor Author

Hm. This worked on the command line.

@hugomrdias hugomrdias requested a review from rvagg February 24, 2021 13:01
Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by me, calling it by Object.prototype is not really necessary since we control the creation of these objects, but whatever.
@hugomrdias I think you'd better review the typing here, I don't think @type {BaseName} is going to work here is it since hasOwnProperty expects a string argument. I'm not sure what should be done, maybe remove the type annotation all together? We don't seem to have any type checking in the scripts here yet that can tell us easily.

@KyleMaas
Copy link
Contributor Author

Fine by me, calling it by Object.prototype is not really necessary since we control the creation of these objects, but whatever.

The Object.prototype stuff was because the linter complained about it if I tried to use hasOwnProperty directly on the object. If there's a better way to do that without failing the linter, go for it.

@rvagg
Copy link
Member

rvagg commented Feb 26, 2021

because the linter

Ahh, right, well that's become the standard "safe" pattern of calling it to ensure that you can call it on any object. Fair enough, it's like being forced to use === when you know == is enough in a particular instance--still good practice.

@hugomrdias
Copy link
Member

LGTM thank you all for the help.

@hugomrdias hugomrdias merged commit d5b670f into multiformats:master Feb 26, 2021
@KyleMaas
Copy link
Contributor Author

Excellent. Thank you!

@KyleMaas KyleMaas deleted the fix-zii-v3 branch February 26, 2021 16:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incompatibility with "zii"
3 participants