Skip to content

Improve documentation on is() and is.convert()() not beein equivalent #12

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

Closed
Roang-zero1 opened this issue Nov 16, 2019 · 3 comments
Closed
Labels
📚 area/docs This affects documentation 💪 phase/solved Post is done

Comments

@Roang-zero1
Copy link
Contributor

is and is.convert()() do not have the same behavior.

The function created through convert() do not have the same results as calling is() directly, even when using the same tests.

I've rewritten all tests from is() to convert()() at Roang-zero1/unist-util-is/tree/convert-test. Running these tests failes 14 tests (e.g. for invalid index).

Steps to reproduce

git clone https://github.com/Roang-zero1/unist-util-is.git
npm install
npm run test

Expected behaviour

From my understanding is() and convert()() should be equivalent.

Actual behaviour

is(node, null, -1) throws and error while

const testNull = convert(null)
testNull(node, -1)

returns undefined.

@Roang-zero1 Roang-zero1 added 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels Nov 16, 2019
@wooorm
Copy link
Member

wooorm commented Nov 17, 2019

Hmm, I don’t think they should be the same, otherwise, why use convert?
Convert is faster on valid input, precisely because it doesn’t test for invalid input, and that’s its use case

@wooorm
Copy link
Member

wooorm commented Nov 17, 2019

Could be clearer in the docs!

@ChristianMurphy ChristianMurphy added good first issue 👋 This may be a great place to get started! 📚 area/docs This affects documentation 🔍 status/open and removed 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels Nov 20, 2019
@Roang-zero1 Roang-zero1 changed the title is() and is.convert()() are not equivalent Improve documentation on is() and is.convert()() not beein equivalent Nov 26, 2019
@ChristianMurphy
Copy link
Member

resolved in #13

@wooorm wooorm added ⛵️ status/released and removed 💪 phase/solved Post is done good first issue 👋 This may be a great place to get started! labels Mar 30, 2020
@wooorm wooorm added the 💪 phase/solved Post is done label Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 area/docs This affects documentation 💪 phase/solved Post is done
Development

No branches or pull requests

3 participants