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

Add unist-util-flat-filter to list of utilities #26

Merged
merged 1 commit into from
Jan 24, 2020
Merged

Add unist-util-flat-filter to list of utilities #26

merged 1 commit into from
Jan 24, 2020

Conversation

crutchcorn
Copy link
Contributor

I found that the filter plugin did not have the functionality I was hoping for, so I created my own plugin complete with tests and published to npm. I added it to the README for discoverability

@wooorm
Copy link
Member

wooorm commented Jan 15, 2020

Nice, thanks! Could you publish the code on GitHub as well, so people can collaborate on it?
And also: you should use is.convert a) if you are also passing in index and parent, and b) reusing it, with this setup is is better.

@crutchcorn
Copy link
Contributor Author

@wooorm Oh goodness! I have no idea why my index.js file was not included in git commits! 😱 I'll be sure to push that. I am using is.convert, however

@crutchcorn
Copy link
Contributor Author

The index.js and index.test.js files have been added to the repo. Sorry once again, t'was an autogenerated gitignore that I forgot about

@wooorm
Copy link
Member

wooorm commented Jan 24, 2020

Sweet!

I am using is.convert, however

Right! What I meant was that, the way you are using is.convert currently, it’s better to use is.
If you want to use is.convert, you should a) convert it in flatFilter, b) pass the new converted function into flatFilterGeneric, and c) make sure that that converted functions get node, index, parent when called.
a) and b) would make it more performant, c) would make sure the user-provided test works exactly the same in your project as all other unist utilities

@wooorm wooorm changed the title Add unist-util-flat-filter to readme.md Add unist-util-flat-filter to list of utilities Jan 24, 2020
@wooorm wooorm merged commit acff392 into syntax-tree:master Jan 24, 2020
@crutchcorn
Copy link
Contributor Author

Oh, I understand better now! I'll be sure to follow up in the library to include the changes as you mentioned. Thanks for the review! 😁

@wooorm
Copy link
Member

wooorm commented Jan 24, 2020

Sweet! Thanks & congrats on your first unist utility! ✨

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

Successfully merging this pull request may close these issues.

2 participants