Open wize-wiz opened 5 years ago
Hi @wize-wiz , I saw you merged one PR I made to this repo recently #60 , At the time when I was facing issues with this package and I made that PR, It worked well for a while, but it falled apart when I started requiring dynamic multiple nested dependencies which required validation and different type comparison operators as you mentioned on this issue,
So today I dusted off a bit the solution I built for my use case at the time, fixed a few issues here and there and published it here: https://github.com/dcasia/conditional-container, however reading some of the issues open on this repo as well past closed issues makes me believe that the path I took on my package is way more bulletproof, as it doesn't require messing much with nova original behavior, it`s simpler and many of the closed and open issues still present on this repository is non existent on my version such as: #81 #40 #62 #32 #31
Of course, I haven't tested it thoroughly besides my use case, but if that helps alleviate the pressure on this repo that has (87k) downloads we could continue development there! read the source code it is simple and straight forward!
Let me know what you think, of course, both packages could coexist but overall if we are trying to solve the same problem best if work together,
ps: I haven't tried to rewrite this package itself cuz it would most certainly break everyone`s current uses case, and the time spent rewriting it and possibly getting delayed on PR review and approval, etc.. could be time wasting.. as I had it already almost done for publishing
Hi @milewski , that's hilarious, we both have rewritten this package mostly the same way. I'm currently writing a 2.0 version of this package but I'm quite unsure because it will break all compatibility.
So I too was playing with the idea to publish it under a different name and try to maintain both packages. I'm not quite finished with 2.0, but it definitely shows potential, specially for the current project I'm working in where we have these absurd logical requirements.
The main difference between your current version and my 2.0 version is that I'm getting rid of the static dependsOn
or your case if
and take an eloquent approach where it's possible to control or
and and
statements by nesting closures. I'm currently writing an approach like so:
// pseudo code, you can nest it indefinitely
DependencyContainer::make([...])
->on(function($container) {
$container
->on('some_field', ['some_value', 'another_value'])
->orOn('another_field', 'last_value')
})
->onChecked('some_boolean');
which produces the following statement:
(some_field = (some_value OR another_value) OR another_field = last_value) AND some_boolean = true
This leads to a far more simpler approach to solve complex logic in a way laravel developers are used to work with queries, e.g. eloquent. Each closure resolves in a nested statement.
But yes, we should somehow work together and build one solid package. I don't think this package (with this package I mean the nova-dependency-container) in its current state has a future and doesn't give the flexibility developers are expecting it to have. Any big changes will also break compatibility, which at some point is unavoidable. So with the right approach and informing everybody an enhanced version of this package is available seems like a good plan.
That`s nice! do you have this v2 WIP online somewhere?
Initially, I tried not to go as deep on the if (AND/OR) because it would most certainly complicate things, especially on the deeply nested dependency feature I'm planning on to release, it`s about 80% done and works like this:
Text::make('A'),
ConditionalContainer::make([
Text::make('B')
ConditionalContainer::make([
Text::make('C')
ConditionalContainer::make([ Text::make('DEEP_NESTED_FIELD') ])->if(...)
])->if(...)
])->if(...)
ConditionalContainer::make([ Text::make('IT WORKS') ])->if('DEEP_NESTED_FIELD', 'foo')
Currently, it's possible to nest ConditionalContainer
as deep as you want but the inner dependency can only depend on the upper fields, like field B
can read the value of A
but can't read value of C
So im unsure how I would tie it up having a more complex way to build the if conditions, as I tried to avoid at all cost to implement the DetailField.vue
and IndexField.vue
, as having custom component wrapping the real fields on these views creates a lot of bugs and incompatibility, if you check my repo I have only implement the FormField.vue
and yet ->onlyOnDetail(), ->onlyOnIndex()
panels and any third party out there should work just fine, only incompatibility with third-party packages might come when they aren't storing their state on the .value property, but it's easy to be fixed by adding another check if component === 'component-name'
, this approach also helped a lot with the validation, it just works without having to create any crazy parsing
But as a consequence, I had to implement the logic to evaluate the if
condition twice (once on the frontend for the form view, and again on the backend for details/index), so having a more complicated if
resolution would require a lot more work which would need to be written in 2 different languages PHP/JS increasing the effort for maintaining the package and creating more surface for bugs...
So im curious to see how you were planning to solve these problems, are you going to resolve everything client-side? at the cost of sending all the data for the client on detail/index view and computing what should be visible client-side?
Hi, @wize-wiz I found a nice library (https://github.com/altef/logipar) to do the logical operation and it worked out of the box without much effort, including the issues I mentioned I would face if I were to take such an approach
now you can literally write this expression:
->if('(some_field = (some_value OR another_value) OR another_field = last_value) AND some_boolean = true')
And it just works
And the nice thing is that logical library works with PHP and JS .. so the duplication of code on client/server is very minimal
Add a third parameter to the
dependsOn
method to supply the comparison operator, by default the operator should be=
.List of comparison operators it should support:
=
by default!=
or<>
not equal==
identical (strict) as in===
>
and>=
greater or greater/equals<
and<=
smaller or smaller/equals><
in betweenExamples:
Only 2.0 or greater:
Only if not
Admin
orModerator
:Inverted would be equal to:
Any thoughts or suggestions are welcome.
Based on the idea of New "dependsOnFalse" dependency option