Skip to content

Required but nullable arguments should be required to be specified in the config #271

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

Closed
ondrejmirtes opened this issue Nov 2, 2021 · 4 comments

Comments

@ondrejmirtes
Copy link
Contributor

ondrejmirtes commented Nov 2, 2021

Let's say we have a constructor that looks like this:

	public function __construct(
		private ?string $singleReflectionFile
	)

And we have a service config that looks like this:

	-
		class: Foo

In this case nette/di will autowire the null value into this constructor parameter. But the the constructor parameter is required for a reason - I want the user to specify it. Omitting it is dangerous because the library can't assume that the user wants null in there, because they might not even know that I want them to specify the value. I want the user to explicitly pass a value.

I realize this is a big BC break, but maybe the behaviour could be opt-in with some feature toggle before making it mandatory in the next major version.

@dg
Copy link
Member

dg commented Nov 2, 2021

That makes sense. I'll try to add a deprecation notice for this situation in version 3.1, which should just warn about the changes in 4.0.

@dg dg closed this as completed in 6c233fb Nov 2, 2021
dg added a commit that referenced this issue Nov 2, 2021
@ondrejmirtes
Copy link
Contributor Author

Awesome, thank you!

dg added a commit that referenced this issue Dec 8, 2021
dg added a commit that referenced this issue Dec 12, 2021
@dg
Copy link
Member

dg commented Dec 14, 2021

I wonder if it shouldn't work that way for objects too? Ie. __construct(private ?Foo $foo)

dg added a commit that referenced this issue Dec 14, 2021
dg added a commit that referenced this issue Dec 14, 2021
@ondrejmirtes
Copy link
Contributor Author

Yeah, if Foo couldn't be autowired, it shouldn't silently pass null in there.

dg added a commit that referenced this issue Dec 14, 2021
dg added a commit that referenced this issue Dec 14, 2021
dg added a commit that referenced this issue Dec 14, 2021
dg added a commit that referenced this issue Dec 14, 2021
dg added a commit that referenced this issue Dec 14, 2021
… is required (for classes, experimental) [Closes #271]
dg added a commit that referenced this issue Dec 14, 2021
dg added a commit that referenced this issue Dec 14, 2021
… is required (for classes, experimental) [Closes #271]
dg added a commit that referenced this issue Dec 14, 2021
dg added a commit that referenced this issue Dec 14, 2021
… is required (for classes, experimental) [Closes #271]
dg added a commit that referenced this issue Dec 15, 2021
dg added a commit that referenced this issue Dec 15, 2021
… is required (for classes, experimental) [Closes #271]
dg added a commit that referenced this issue Dec 15, 2021
dg added a commit that referenced this issue Dec 15, 2021
… is required (for classes, experimental) [Closes #271]
dg added a commit that referenced this issue Dec 15, 2021
dg added a commit that referenced this issue Dec 15, 2021
… is required (for classes, experimental) [Closes #271]
dg added a commit that referenced this issue Dec 15, 2021
dg added a commit that referenced this issue Jan 19, 2022
dg added a commit that referenced this issue Feb 23, 2022
dg added a commit that referenced this issue Mar 10, 2022
dg added a commit that referenced this issue Mar 10, 2022
dg added a commit that referenced this issue Apr 20, 2022
dg added a commit that referenced this issue Jun 1, 2022
dg added a commit that referenced this issue Jun 2, 2022
dg added a commit that referenced this issue Oct 13, 2022
dg added a commit that referenced this issue Oct 13, 2022
dg added a commit that referenced this issue Nov 2, 2022
dg added a commit that referenced this issue Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants