Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

!!!TASK: Remove support for Flow's custom class loader #2775

Merged
merged 2 commits into from
Mar 28, 2022

Conversation

kdambekalns
Copy link
Member

@kdambekalns kdambekalns commented Mar 25, 2022

This drops support for the Flow ClassLoader – except for functional
testing. There it's still used to load test classes & fixtures.
If you still used the FLOW_ONLY_COMPOSER_LOADER environment variable to fallback to the old Flow autoloader, you need to update your package to use proper composer autoloading via autoload section.

This drops support for the Flow ClassLoader – except for functional
testing. There it's still used to load test classes & fixtures.
@kdambekalns kdambekalns requested review from kitsunet and albe March 25, 2022 23:14
@kdambekalns kdambekalns self-assigned this Mar 25, 2022
@kdambekalns
Copy link
Member Author

Getting the tests to run without the class loader would be awesome, too…

@mficzel mficzel self-requested a review March 26, 2022 11:53
@albe
Copy link
Member

albe commented Mar 26, 2022

See also #2771 - I think we can get rid of the testing case if we manage to add autoload-dev section to the root composer manifest in the dev-distribution

@mficzel
Copy link
Member

mficzel commented Mar 28, 2022

@albe that is not so easy as this would be required in all projects that use our unit tests as base. The clean way would be to extract the testcase base classes into seperate package that is dev-dependency. That way the tests for each package could be executed without depending on other packages tests.

Copy link
Member

@mficzel mficzel left a comment

Choose a reason for hiding this comment

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

This one causes an error in Neos that should be addressed. Do not get yet why that happens exactly.

Required class "Neos\CliSetup\Command\SetupCommandController" could not be loaded properly for reflection from "Neos\CliSetup\Command\SetupCommandController".

The error may be a good sign and even correct.

@albe
Copy link
Member

albe commented Mar 28, 2022

The clean way would be to extract the testcase base classes into seperate package that is dev-dependency. That way the tests for each package could be executed without depending on other packages tests.

But how would that make autoloading for Neos\Foo\Tests\X classes work? Those namespaces then need to be listed in the autoload section of all packages that contain tests, no? Which means cluttering the production autoloader with testing namespaces?

@mficzel
Copy link
Member

mficzel commented Mar 28, 2022

Maybe this will solve this problem neos/neos-development-collection#3680

Copy link
Member

@mficzel mficzel left a comment

Choose a reason for hiding this comment

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

With this change things broke that should not have worked in the first place. Fine by testing but not fully understanding.

Copy link
Member

@albe albe left a comment

Choose a reason for hiding this comment

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

Fine as is, let's tackle the bigger fish for next major

@albe albe changed the title TASK: Remove support for Flow's custom class loader !!!TASK: Remove support for Flow's custom class loader Mar 28, 2022
@mficzel mficzel merged commit 7bcde58 into neos:master Mar 28, 2022
@kdambekalns kdambekalns deleted the task/remove-custom-classloader branch March 28, 2022 16:24
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.

3 participants