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

refactor(filter): decouple from lib #217

Merged
merged 17 commits into from
Jan 26, 2016
Merged

Conversation

Zertz
Copy link
Collaborator

@Zertz Zertz commented Jan 5, 2016

Filters are now stored in the core module, which will eventually allow the resource filter to be removed from core.

I have also cleaned up the interface and the functions themselves are loosely coupled, so it is now possible to split them into different modules.

Finally, the constructor is now "pure", as it no longer mutates its parameters.

As a bonus, filtering now happens right before output, allowing access to the full document within post middleware and closing #186. This could possibly be a breaking change for some users.

Known issues

Skip tests relying on side effects
Update tests with new filter constructor
Add internal API docs (generate with jsdoc3)
@Zertz Zertz force-pushed the refactor-resource-filter branch from 07ade6e to ea0ea48 Compare January 7, 2016 15:25
@Zertz
Copy link
Collaborator Author

Zertz commented Jan 18, 2016

@florianholzapfel Can you have a look at this? This is mostly paving the way for further improving the filter.

@florianholzapfel
Copy link
Owner

@Zertz looks good to me

@Zertz
Copy link
Collaborator Author

Zertz commented Jan 19, 2016

Cool! Might be nice to implement @smirea's idea in #224 at the same time since both might be a breaking change for some users.

@Zertz
Copy link
Collaborator Author

Zertz commented Jan 25, 2016

Alright, I extracted removeValueAtPath and getModelAtPath from the filter and moved them into their own repositories: weedout and mongoose-detective. @florianholzapfel Feel free to have another look and merge!

florianholzapfel added a commit that referenced this pull request Jan 26, 2016
@florianholzapfel florianholzapfel merged commit 3ae8f1d into master Jan 26, 2016
@florianholzapfel
Copy link
Owner

great work!

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

Successfully merging this pull request may close these issues.

2 participants