Skip to content

Field Type Registry: Share custom types across instances#34

Merged
nicolas-jaussaud merged 5 commits into
mainfrom
fix/field-registry-shared-types
May 27, 2026
Merged

Field Type Registry: Share custom types across instances#34
nicolas-jaussaud merged 5 commits into
mainfrom
fix/field-registry-shared-types

Conversation

@nicolas-jaussaud
Copy link
Copy Markdown
Contributor

Hi @zinigor!

I believe there is a small issue with the custom field types registration currently

According to this example from the documentation, FieldTypeRegistry instances should share custom types:

// Create and configure a shared registry
$registry = new \Tangible\DataView\FieldTypeRegistry();
$registry->register_type('phone', [...]);

// Use it for multiple views
$view1 = new DataView(['fields' => ['phone' => 'phone'], ...]);
$view2 = new DataView(['fields' => ['mobile' => 'phone'], ...]);

However, it does not seem to be the case currently as each instance hold its own types (here)

This behavior prevents the use of custom fields, as DataView instantiate its own FieldTypeRegistry instance (here, inside the constructor) which will trigger a fatal error when reaching this part if a field type is not registered

I switched types to a static attribute, as it seems to be the expected behavior and was the easiest way to fix the issue without introducing syntax changes

If you don't want to change FieldTypeRegistry, I can also change this PR to pass a FieldTypeRegistry instance as a second (and optional) parameter of DataView:

$registry = new \Tangible\DataView\FieldTypeRegistry();
$registry->register_type('phone', [...]);

// Shared custom types
$view1 = new DataView(['fields' => ['phone' => 'phone'], ...], $registry);
$view2 = new DataView(['fields' => ['mobile' => 'phone'], ...], $registry);

// Only default types
$view2 = new DataView(['fields' => ['mobile' => 'phone'], ...]);

@nicolas-jaussaud nicolas-jaussaud requested a review from zinigor May 19, 2026 18:00
Copy link
Copy Markdown
Contributor

@zinigor zinigor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I can see how the doc is not describing the functionality well. I really like your suggestion about making an optional registry parameter. It seems that currently the DataView is meant to work as a single top level instance, and I really like the dependency injection-ish approach that this allows us to use in the future. I think it would be enough, because everything else DataView initializes in the constructor seems to be reusable and depends on the registry and the passed configuration.

@nicolas-jaussaud
Copy link
Copy Markdown
Contributor Author

Alright!

I updated the PR, we should now be able to define a custom FieldTypeRegistry when creating a DataView:

$registry = new FieldTypeRegistry();
$registry->register_type( 'phone', [...] );

$view = new DataView(
    [ 'fields' => [ 'phone' => 'phone' ], ... ],
    $registry
);

@nicolas-jaussaud nicolas-jaussaud requested a review from zinigor May 26, 2026 13:39
zinigor
zinigor previously approved these changes May 27, 2026
Copy link
Copy Markdown
Contributor

@zinigor zinigor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, let's go with this approach!

@nicolas-jaussaud nicolas-jaussaud merged commit 8a026c9 into main May 27, 2026
3 checks passed
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