-
-
Notifications
You must be signed in to change notification settings - Fork 437
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
WIP - Add wildcard to inject directive #2280
base: master
Are you sure you want to change the base?
Changes from 13 commits
eedb6ea
78a92ce
7226a09
acb91bb
5f1c74b
b607c28
8ff0340
8b4ffec
eea6145
d20a332
60f8834
e163ae7
4a49468
4fc5b9c
95299b0
86ee821
9f17d55
503347c
3ae756e
7bb4ffb
36e3e0e
d0a1cd7
829172f
a42c22e
42b907c
a0e55d8
ead7514
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,8 @@ | |
|
||
namespace Nuwave\Lighthouse\Execution\Arguments; | ||
|
||
use GraphQL\Error\Error; | ||
|
||
class ArgumentSet | ||
{ | ||
/** | ||
|
@@ -61,19 +63,42 @@ public function has(string $key): bool | |
|
||
/** | ||
* Add a value at the dot-separated path. | ||
* | ||
* Works just like @see \Illuminate\Support\Arr::add(). | ||
* Asterisks may be used to indicate wildcards. | ||
* | ||
* @param mixed $value any value to inject | ||
*/ | ||
public function addValue(string $path, $value): self | ||
public function addValue(string $path, mixed $value): self | ||
{ | ||
$argumentSet = $this; | ||
$keys = explode('.', $path); | ||
|
||
self::applyValue($keys, $value, $argumentSet); | ||
|
||
return $this; | ||
} | ||
|
||
/** | ||
* @param array<string> $keys | ||
* @param mixed $value any value to inject | ||
* @param ArgumentSet $argumentSet | ||
*/ | ||
private static function applyValue(array $keys, $value, ArgumentSet $argumentSet): void | ||
{ | ||
while (count($keys) > 1) { | ||
$key = array_shift($keys); | ||
|
||
if ($key === '*') { | ||
if (!is_array($argumentSet)) { | ||
throw new Error('Asterisk `*` must target an array in the list.'); | ||
} | ||
|
||
foreach ($argumentSet as $argument) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if we should throw an error with it's not an array here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think so, yeah. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've updated it now. Not sure if i used the correct exception. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |
||
self::applyValue($keys, $value, $argument); | ||
} | ||
|
||
return; | ||
} | ||
|
||
// If the key doesn't exist at this depth, we will just create an empty ArgumentSet | ||
// to hold the next value, allowing us to create the ArgumentSet to hold a final | ||
// value at the correct depth. Then we'll keep digging into the ArgumentSet. | ||
|
@@ -89,8 +114,6 @@ public function addValue(string $path, $value): self | |
$argument = new Argument(); | ||
$argument->value = $value; | ||
$argumentSet->arguments[array_shift($keys)] = $argument; | ||
|
||
return $this; | ||
} | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes me wonder if we should make the injection optional. In this example, I could see
input.tasks.create
being optional, perhaps withinput.tasks.update
being another possible argument that should also be injected into if present.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the same "issue" could exists with just dot notation. But it would make sense to discard it if an array does not exists at the asterisk segment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With just dot notation, we force the value at the defined position - and create intermediary inputs if necessary. I guess this could not be desired in every case, but it is how it currently works. At least it is well defined.
In the case of an array, we have the problem that it is less clear what to insert. What number of arguments at the desired position should be added? 1 or another number? I would argue it should always be 0 - that being equivalent to injecting nothing at all.