Compatibility with 7.4 preloading

[brendt]

There's an issue with this package when using PHP 7.4's preloading. More specifically with this file: https://github.com/nette/di/blob/master/src/compatibility.php

Because these class aliases are defined unconditionally, they will throw "previously declared class warnings" if the file was preloaded.

More info: https://mobile.twitter.com/nikita_ppv/status/1208025415253659648

[dg]

I dont understand what to do.

[mabar]

Extend class to only load it if used could work, I guess

namespace Nette\DI;
/**
 * @deprecated
 */
class ServiceDefinition extends \Nette\DI\Definitions\ServiceDefinition {}

[JanTvrdik]

Defining the aliases conditionally should work, i.e.

if (!class_exists(Nette\DI\Config\IAdapter::class, false)) {
   class_alias(Nette\DI\Config\Adapter::class, Nette\DI\Config\IAdapter::class);
}

...

[dg]

@JanTvrdik How is it possible that a class IAdapter exists?

[mabar]

It's a preloading feature. All preloaded classes and functions are available across requests, so you don't need to load them every single time.
https://stitcher.io/blog/preloading-in-php-74

[dg]

So the problem is that the code on the page https://stitcher.io/blog/preloading-in-php-74 loads all the files in repo and then one file compatiblity.php is loaded by Composer?

Guys, I really don't like issues written as a puzzle that I have to solve to understand the task.

[dg]

Above all, loading all *.php files from the vendor/xxx directory is a short-sighted idea, because you are not sure that scripts do not have side effects (like script in nette/di has). Or this one https://github.com/nette/tracy/blob/master/tools/create-phar/create-phar.php.

Condition is not a solution, fix a preloading script. For example let the Composer create a list of classes and preload them:

composer dumpautoload --classmap-authoritative

$list = require 'autoload_classmap.php';
$list = array_filter($list, fn($file) => strpos($file, '/vendor/xxx/') !== false);
...

[brendt]

Hi David, thanks for looking into this. When I wrote this issue I was short on time and I assumed preloading was already a known concept for most open source maintainers.

So let me clarify: preloading is a core feature as of PHP 7.4, and has nothing to do with composer. It can significantly reduce request startup time, especially for projects using frameworks. While optimised preload lists are the ideal solution, they don't offer much more performance gain compared to a "dumb" preloading list which preloads all classes of a given framework. You can read about these benchmarks on the composer repository: composer/composer#7777 (comment)

My preload script will try to preload all of Laravel's classes, and it so happens that, because of dev dependencies, nette/bootstrap is also loaded (required by phpstan/phpstan).

Obviously I can probably work around the issue, but I figured it would be a small effort to add a class_exists call. I'm perfectly fine with sending a PR myself.

This probably won't be the last issue being submitted here by people who want to use a simple preload script to preload their framework, having phpstan installed and getting an error. Most of these people will submit an issue here, requiring some of your time. I think it's in both our interests to prevent this error from happening again — even though there might be other solutions. Being an open source maintainer myself, I prefer to make things fool proof on my end, so that I don't have to deal with the same issues over and over again.

[dg]

I think I understand how preloading works. But it is not about preloading and it is not about Composer. It is about some specific code that preloads classes. And I don't know the code. I think there is no standard preloader.

The preloader mentioned by @mabar has issue - it simply loads all the files. It does not scale. Because loading all files is not a good idea at all. There will be more and more libraries with the same issues. Libraries with examples or tests or make-scripts inside its repo. Preloading script can't just load all them files. You can't even write to all library authors: fix it. Preloading script must be fixed.

And I will be happy to help find a way to fix that script. But since I'm a programmer who wants to do things right, I want to fix a bug, not its consequences.

[brendt]

We're not in the same page, and it's clear that you don't want to change your mind. I'll close this issue and work around it. Thanks for your time.

[dg]

I probably don't change the idea that loading all the *.php in a repository is a bad idea (do you really think it's good?), but I'm trying to be helpful in finding another general solution.

[brendt]

Which files are loaded or not is not the issue though. compatibility.php is configured to be autoloaded automatically: https://github.com/nette/di/blob/master/composer.json#L36

We could write preload scripts that don't use composer's autoloader, and manually resolve all dependencies, but that's a lot of work for frameworks, and not worth the time.

The fact is simply this: if someone wants to use preloading combined with composer's autoloader (which does 99% of the work for you), and has a dependency on nette/di, they won't be able to do any preloading at all.

If you're fine with that, then keep the code as is; but since it's a simple fix (semantically more correct in my opinion), I don't see any reason not to add it.

[brendt]

FYI the solution shared by @JanTvrdik wasn't 100% correct, since Nette\DI\Config\IAdapter is an interface. This is a working solution:

if (!interface_exists(Nette\DI\Config\IAdapter::class)) {
    class_alias(Nette\DI\Config\Adapter::class, Nette\DI\Config\IAdapter::class);
}

if (!class_exists(Nette\DI\Statement::class)) {
    class_alias(Nette\DI\Definitions\Statement::class, Nette\DI\Statement::class);
}

if (!class_exists(Nette\DI\ServiceDefinition::class)) {
    class_alias(Nette\DI\Definitions\ServiceDefinition::class, Nette\DI\ServiceDefinition::class);
}

[dg]

The fact is simply this: if someone wants to use preloading combined with composer's autoloader

What is to use preloading? Do you mean to use preloading with Preloaded from stitcher.io/blog? If so, there is a conflict between the script Preloader and nette/di. But not between PHP 7.4 preloading feature and nette/di. (If not, please explain me how nette/di disallows preloading generally, I do not understand that).

I will further assume that this is a problem between the class Preloader and nette/di. Caused by loading the compatiblity.php file that has a side effect.

This file contains no class so I am asking why Preloader is loading it?

You said because it is "dumb" and preloads all classes of a given framework.

If Preloader does not load this file, it will solve the problem.

So the question is: what to fix? Preloader or nette/di.

I say unambiguously: Preloader. Because there are lots of other libraries where the same problem arises. And it's better to repair the Preloader than to ask the authors of hundreds of libraries to modify their libraries for Preloader. Not for feature preloading, but for one class named Preloader.

For example: inside Nette Tester https://github.com/nette/tester/tree/master/src there is file tester.php with die(). If someone wants to use Preloader (not preloading) and has a dependency on nette/tester, they won't be able to do it at all. Whose fault is it?

Which files are loaded or not is the issue.