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.

Sign up for GitHub

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

Jump to bottom

extraDependencies for createQuery and createJsonQuery #372

Closed
wants to merge 13 commits into from
Closed

extraDependencies for createQuery and createJsonQuery #372

wants to merge 13 commits into from

Conversation

NazariiShvets
Copy link
Contributor

fixes: #350

@NazariiShvets NazariiShvets marked this pull request as ready for review September 23, 2023 01:04
@NazariiShvets NazariiShvets changed the title extraDependecies for createQuery and createJsonQuery extraDependencies for createQuery and createJsonQuery Sep 23, 2023
igorkamyshev
igorkamyshev requested changes Sep 23, 2023
View reviewed changes
Copy link
Owner

@igorkamyshev igorkamyshev left a comment

Choose a reason for hiding this comment

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

Thanks for contribution 😇

  1. I am not sure if this change is good. Let's finish discussion in the issue first.
  2. We do not accept any features without docs and changesets. Please, refer to the contributions guide in the readme.

@NazariiShvets
Copy link
Contributor Author

NazariiShvets commented Sep 23, 2023 •
edited

There is no way to provide custom sourced fields using createQuery and createJsonQuery. It PR allows to pass stores as relation for query to handle sourced fields relation side-effects

@igorkamyshev
Copy link
Owner

Check internal implementation of createQuery. You can write something like this in your code.

@NazariiShvets
Copy link
Contributor Author

NazariiShvets commented Sep 23, 2023 •
edited

Sure

I also can fork whole farfetched to my codebase if i need 1 simple extra parameter. But i dont want all of that complexity and extra maintaining cost for new releases, so i decided to contribute to main repo

If u think its wrong fit just say it and close PR/issue. I will start looking to find a way to deal with my cases in my codebase instead

@igorkamyshev
Copy link
Owner

Sorry, without strong motivation and any documentation I simply do not get why this feature has to be in core.

Please, provide proper RFC and documentation and reopen PR with it 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@igorkamyshev igorkamyshev igorkamyshev requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add a recipe about custom factories for specific needs
2 participants
@NazariiShvets @igorkamyshev