Problem/Motivation

We're still following through on #1826054: [Meta] Expose Drupal Components outside of Drupal

And we need to be able to test our components independent of Drupal core.

As a very small first step, we need to allow each component to specify it's PSR-4 autoload and autoload-dev sections per composer.json file.

We want each component to define its own namespace.

Since we're merging all of the components' composer.json files during composer install time, their autoload and autoload-dev sections can be used to generate the PSR-4 mappings for each individual component.

Currently, core/composer.json forces all components to be PSR-4 namespaces, which disallows moving source files into subdirectories.

This will open the door to moving the source files of those components into a src/ directory, and also moving the components' test files into a tests/ directory.

Proposed resolution

Remove the Drupal\Components\ namespace from core/composer.json's autoload section.

Add Drupal\Components\[Component]\ PSR-4 namespace declarations per component's composer.json file. Each composer.json file already does this.

Move the component's source files to a src/ directory.

Remaining tasks

Move component tests to a tests/ directory.

Add composer.json autoload-dev section for the tests/ directory.

User interface changes

API changes

Data model changes

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

Mile23 created an issue. See original summary.

Mile23’s picture

Status: Active » Needs review
FileSize
3.22 KB

Here's a POC. It only moves Annotation to a src/ directory, but illustrates that autoloading already works because each component already specifies autoload. :-)

I hadn't remembered that for some reason, so this issue could probably use some re-scoping.

Mile23’s picture

Issue summary: View changes
Mile23’s picture

Added #2943856: Move component tests to component directories, mostly so I could note that we need to move Transliteration/src/data to tests/data when the time comes, and change the tests to use it.

This is all the components moved to src/ with updated composer.json. Also updating core/composer.json to reflect the components' new-found self-naming agency.

Status: Needs review » Needs work

The last submitted patch, 4: 2943842_4.patch, failed testing. View results

Mile23’s picture

Let's hurry up and get rid of PHP 5 please. :-)

Mile23’s picture

Status: Needs work » Needs review
alexpott’s picture

I'm not sure this is truly necessary... it's not what Symfony does. They do
One problem is that the regular runtime autoloader should not be able to autoload tests so each composer.json is going to need something like

    "autoload": {
        "exclude-from-classmap": [
            "/Tests/"
        ]
   },

The one bit of fun is that our test infra is rather bizarre in that it autoloads tests.... however I think I have a solution for that. See https://www.drupal.org/project/drupal/issues/2943856

alexpott’s picture

I actually think we could merge this issue into the https://www.drupal.org/project/drupal/issues/2943856 rather than do it in two separate steps if we don't have to move all the code into src.

alexpott’s picture

Hmmm... but actually maybe the test move issue should be a meta and we should move each component separately as we should add a PHPUnit.xml.dist to each component and change or maybe removing the TESTING.txt since it is probably overkill once the phpunit.xml.dist is in place.

So let's sort out the autoloader / component composer.json's here. Slightly different tack so no interdiff. It is basically #6 without the file moves and adding ignoring of any Tests folders (which are yet to exist but that does not matter).

Mile23’s picture

One problem is that the regular runtime autoloader should not be able to autoload tests

Correct. :-)

When we remove the Drupal\Components\ namespace from core/composer.json's PSR-4 section, that lets the components define their own PSR-4 namespace.

Try it: Apply the patch in #6 (or really #10, either way). Remove an autoload section from one of the component's composer.json. Do a composer dumpautoload. Then run the tests: ./vendor/bin/phpunit -c core/ core/tests/Drupal/Tests/Component/ You'll see that the autoloading for that component isn't available.

I actually think we could merge this issue into #2943856: Move component tests to component directories

I think we want the tests to remain where they are so we can be consistent with the move here, and then move the tests as a separate step.

+++ b/core/lib/Drupal/Component/Annotation/composer.json
@@ -15,6 +15,9 @@
       "Drupal\\Component\\Annotation\\": ""
...
+    "exclude-from-classmap": [
+      "/Tests/"

Given that each component can define its own PSR-4, we can leave the classes at the top level directory, or put the source in a src/ directory. If we put it in a src/ directory, we don't have to exclude-from-classmap for tests later on.

alexpott’s picture

@Mile23 but we're already inside core's lib directory. Let's follow Symfony's lead and not add extra unnecessary subdirectories. Also not moving the code is just less change (that is not necessary).

Mile23’s picture

We are inside core/lib, but core/composer.json says this:

    "autoload": {
        "psr-4": {
            "Drupal\\Core\\": "lib/Drupal/Core",
            "Drupal\\Component\\": "lib/Drupal/Component",
            "Drupal\\Driver\\": "../drivers/lib/Drupal/Driver"
        },

So the only PSR-4 autoloading for core is lib/Drupal/Core/ after we remove the entry for lib/Drupal/Component.

Either way is fine, just less hassle later with src/ if we're going to move tests around, since we'll be able to rely on the tests in this issue.

Mile23’s picture

Status: Needs review » Needs work
+++ b/core/composer.json
@@ -192,13 +192,10 @@
-            "lib/Drupal/Component/Utility/Timer.php",
-            "lib/Drupal/Component/Utility/Unicode.php",

+++ b/core/lib/Drupal/Component/Annotation/composer.json
@@ -15,6 +15,9 @@
+    "exclude-from-classmap": [
+      "/Tests/"

If we're not going to use src/ then we don't really need to add an exclude for a non-existent directory. In that case we don't really need to change any of the composer.json files other than core.

The only change we might need to a component compser.json would be to add Timer and Unicode to the classmap in Utility's composer.json if we think that's worth the performance improvement.

alexpott’s picture

Fair enough we can add the tests to the exclude when we move them. The files were first hard coded to the class loader in #2253593: Stop classloader searching filesystem for classes before drupal_classloader() is called - i'm not sure we even need to change that here. So this patch becomes a 1 liner :)

alexpott’s picture

Status: Needs work » Needs review
FileSize
407 bytes
Mile23’s picture

Status: Needs review » Reviewed & tested by the community

I'd give a little pushback on removing the classmap lines from core and putting them in Utility, but it's not a problem until it becomes one at some point. :-)