Skip to content
This repository was archived by the owner on Feb 13, 2026. It is now read-only.
/ core Public archive

Comments

Wrap combined file content with global namespace#8530

Closed
dmolineus wants to merge 2 commits intocontao:developfrom
dmolineus:fix/namespaces-in-combined-files
Closed

Wrap combined file content with global namespace#8530
dmolineus wants to merge 2 commits intocontao:developfrom
dmolineus:fix/namespaces-in-combined-files

Conversation

@dmolineus
Copy link
Contributor

Since Contao supports callables for dca callbacks and PHP 5.5 introduced the Class:class constant there are valid cases using the use statement for namespaced classes in the dca and config files.

At the moment you cannot use the use statement because of the danger that the combined files could have duplicate use statements. This pull request fixes it by wrapping each file content in a namespace notation.

So it would get possible to use following code safely:

DCA file

<?php 
// dca/tl_content.php
use Vendor\Namespace\GetTemplatesCallback;

$GLOBALS['TL_DCA']['tl_content']['fields']['customTemplate']['options_callback'] = new GetTemplatesCallback('ce_custom');

Config file

<?php 
// config/config.php
use Vendor\Namespace\CustomElement;
$GLOBALS['TL_CTE']['text']['custom'] = CustomElement::class;

@leofeyer
Copy link
Member

What is wrong with $GLOBALS['TL_CTE']['text']['custom'] = 'Vendor\Namespace\CustomElement';?

@aschempp
Copy link
Member

@leofeyer that's a different case, you're referring to a string which always must be used with FQCN, but @dmolineus is referring to calling code in the respective files.

@leofeyer
Copy link
Member

I wanted to point out that it is not necessary to use use statements in DCA files. As we all agree, DCA files should not have contained classes and functions in the first place, therefore it seems wrong to me to implement stuff that further encourages developers to do so.

Our goal should be to reduce PHP usage in DCA files, so we can e.g. use YAML files in the future.

@dmolineus
Copy link
Contributor Author

What is wrong with $GLOBALS['TL_CTE']['text']['custom'] = 'Vendor\Namespace\CustomElement';?

Nothing wrong with that. The benefit of CustomElement::class would be that renaming or moving the class the reference would automatically updated.

And yes, the class does not have to be loaded to use the ::class. In PHP ::class also works for non existing classes.

@dmolineus
Copy link
Contributor Author

I wanted to point out that it is not necessary to use use statements in DCA files. As we all agree, DCA files should not have contained classes and functions in the first place, therefore it seems wrong to me to implement stuff that further encourages developers to do so.

Using the ::class notation is a very useful feature. Allowing use statements would increase the readability of the dca files when using multiple callback definitions.

Furthermore Contao supports callable here since 3.2(?). For using class names inside anonymously functions using the use statement would be helpful.

Our goal should be to reduce PHP usage in DCA files, so we can e.g. use YAML files in the future.

As long as the PHP are used, standard PHP features should be supported as well. Detecting name refences by IDEs when renaming or moving classes is one benefit from it (and a drawback of YAML files).

@Toflar
Copy link
Member

Toflar commented Oct 19, 2016

I'm +1 on this PR.

@discordier
Copy link
Contributor

I am also 👍

I have experienced the described issue in the past and had to remove the imports from config.php files.

leofeyer added a commit to contao/core-bundle that referenced this pull request Oct 21, 2016
@leofeyer leofeyer added this to the 4.3.0 milestone Oct 21, 2016
@leofeyer leofeyer self-assigned this Oct 21, 2016
@leofeyer
Copy link
Member

Implemented in contao/core-bundle@6669a09.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants