Skip to content

Allow Schema elements as default values#12

Merged
dg merged 1 commit into
nette:masterfrom
mabar:patch-1
Oct 31, 2019
Merged

Allow Schema elements as default values#12
dg merged 1 commit into
nette:masterfrom
mabar:patch-1

Conversation

@mabar

@mabar mabar commented Oct 12, 2019

Copy link
Copy Markdown
Contributor
  • 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

dg commented Oct 22, 2019

Copy link
Copy Markdown
Member

Great. Can you add test?

@mabar

mabar commented Oct 22, 2019

Copy link
Copy Markdown
Contributor Author

Done

@dg

dg commented Oct 31, 2019

Copy link
Copy Markdown
Member

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

dg commented Dec 31, 2020

Copy link
Copy Markdown
Member

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

mabar commented Dec 31, 2020

Copy link
Copy Markdown
Contributor Author

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