Skip to content

Conversation

@mabar
Copy link
Contributor

@mabar mabar commented Oct 12, 2019

  • new feature?
  • BC break? no

We have some composite extensions which simulates compiler passes. They can be disabled so configuration looks like this Expect::anyOf(false, SubExtension::createSchema()).

AnyOf does not match default value null (empty configuration) same way as if we directly use Expect::structure (no AnyOf) so that structure is not used.

Current workaround looks like this

	public function getConfigSchema(): Schema
	{
		$subExtensionSchema = SubExtension::createSchema();

		return Expect::structure([
			'subExtension' => Expect::anyOf(false, $subExtensionSchema)->default($subExtensionSchema->completeDefault(new Context())),
		]);
	}

With suggested change would be simplified call to default to default($subExtensionSchema) and it also allows any schemas used as default values.

Alternative solution would be match Structure with null value in AnyOf but I am not sure if it's a correct behavior as it would depend on order of structure and null if both used and maybe break also some other combinations.

@dg
Copy link
Member

dg commented Oct 22, 2019

Great. Can you add test?

@mabar
Copy link
Contributor Author

mabar commented Oct 22, 2019

Done

@dg
Copy link
Member

dg commented Oct 31, 2019

Thanks

@dg dg merged commit 554cb39 into nette:master Oct 31, 2019
dg pushed a commit that referenced this pull request Oct 31, 2019
dg pushed a commit that referenced this pull request Oct 31, 2019
dg pushed a commit that referenced this pull request Oct 31, 2019
@dg
Copy link
Member

dg commented Dec 31, 2020

Wouldn't it be better to use this solution, ie method that sets the first item as the default?

   Expect::anyOf(
        $subExtensionSchema
        false, 
   )->firstAsDefault(),

@mabar
Copy link
Contributor Author

mabar commented Dec 31, 2020

Imho it's better to keep that behavior hidden. If I implemented this now, I would throw exception for any Schema element except Structure, because behavior of inheriting e.g. null from Expect::string() is mostly confusing and useless.

But yeah, may be a nice shortcut.

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

Successfully merging this pull request may close these issues.

2 participants