Skrol29 / tinybutstrong

TBS is a PHP template engine for pro and beginners. Only 1 class with few methods properties, but it can do may things for any text templates, including HTML and XML. The only engine that enables W3C compliant templates. It has many plugins including OpenTBS.
http://www.tinybutstrong.com
60 stars 18 forks source link

Usage of global constants breaks apps using PHP 7.4s class preloading #20

Open spackmat opened 3 years ago

spackmat commented 3 years ago

Hi, I enabled PHP class preloading for my Symfony based app. But when it comes to creating files with TBS/OpenTBS, I get an error "Undefined constant 'TBS_INSTALL'". After digging in, I realized that preloading and global constants defined at runtime with define() won't work together: they are simply not defined when their class is preloaded.

Can anyone confirm that? Any ideas to solve that (besides refactoring the constants-system of TBS/OpenTBS creating a BC break)?

I tried to exclude the (service) class using the TBS_INSTALL constant from preloading, but that doesn't solve the problem (as it and the TBS classes are still preloaded as a dependency).

How to reproduce

Just try to use TBS/OpenTBS in an application with PHP preloading. In opcache_get_status()['preload_statistics']['classes'] you get a list of preloaded classes where clsTinyButStrong, clsTbsDataSource and clsTbsLocator are included, provoking the error.

roxblnfk commented 3 years ago

@Skrol29 what do you think about releasing a new major version of the TBS package, which will support minimum PHP 7.4?

This can be done with preservation of API and compatibility (using a decorators). This will have modern code that adheres to PSR, with phpunit 9 for testing, etc.

Skrol29 commented 3 years ago

Hi,

Sorry I need more time to reproduce this issue since Preloading is not available on PHP for Windows for now. (shame on me, I use Windows). My Linux environment is far from here.

It’s surprising that constants are ignored with preloading. The documentation is unclear on this point :

Any symbols (functions, classes, etc.) in those files will then become globally available

I wonder if PHP 8 has the same behavior.

Nevertheless I agree the next major TBS version should avoid named constants (define) and use Class constants instead.

A waiting solution could be to have TBS and OpenTBS constants defined only once dynamically at the first instantation of the class. Does such a solution sounds ok for you ?

Skrol29 commented 3 years ago

@Skrol29 what do you think about releasing a new major version of the TBS package, which will support minimum PHP 7.4? This can be done with preservation of API and compatibility (using a decorators). This will have modern code that adheres to PSR, with phpunit 9 for testing, etc.

Having a major version suitable for PHP 7 and modern coding is a very nice idea which I often think of. I also think about leaving unused features (such as Assigned merging for which I have zero feedback) ; add new a barrel feature for the Engine in order to have it processing sub-templates in the same way as OpenTBS ; and some few renaming. So it is not clear for me to have all this in one major version or several.

spackmat commented 3 years ago

@Skrol29 I was also surprised, but it is consequent as define() is a function called at runtime and preloading doesn't run any code, but only compiles the symbols and keeps them in memory. So code in preloaded files outside those symbols will never run, because the actual files are not required/included anymore.

A waiting solution could be to have TBS and OpenTBS constants defined only once dynamically at the first instantation of the class. Does such a solution sounds ok for you ?

You mean inside the constructor of the main classes with a check like if (!defined('MY_GLOBAL_CONSTANT')){define('MY_GLOBAL_CONSTANT', 'MY_VALUE');}? Could work.

My devsystem is also on windows, so I see that problem only in staging/production. Nice surprise. ;)

Besides that: A new major version with contemporary coding is always a good idea.