codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.35k stars 1.9k forks source link

Bug: Double initializing of class #3855

Closed jbotka closed 3 years ago

jbotka commented 3 years ago

Describe the bug Double initializing of the config class. Problem is that I am extending config class Filters from module. I have setup that I have shared configs between all apps and I am extending main config file located in shared folder by apps config. But happens that auto discovery is trying to make second initializing run in my shared folder a that makes fatal error for initializing already initialized class.

CodeIgniter 4 version 4.0.4 (develop)

Affected module(s) Config

Expected behavior, and steps to reproduce if appropriate Auto discovery should no try initialize already initialized class. There should be check to prevent it.

Context

paulbalandan commented 3 years ago

Can you describe more of your issue?

As I understand, you are extending Config\Filters but how are you doing it exactly?

jbotka commented 3 years ago

Can you describe more of your issue?

As I understand, you are extending Config\Filters but how are you doing it exactly?

class Filters extends \SharedNamespace\Config\Filters

I am not sure if this runs first initializing or auto-discovery it runs first.

paulbalandan commented 3 years ago

Please provide more information to help us solve your issue.

  1. Are you using the released 4.0.4 version or the latest develop?
  2. Do you have a test case that can reproduce the problem?
jbotka commented 3 years ago

Please provide more information to help us solve your issue.

  1. Are you using the released 4.0.4 version or the latest develop?
  2. Do you have a test case that can reproduce the problem?
  1. It is mentioned in the first "4.0.4 (develop)" so latest
  2. Case provided. make module and try to extend config file from the module like I provided :)

I am not sure which runs first. Either should be check in auto-discovery or if auto-disovcery has such thing, then autodisovery should run after initial run so there will be no occurrence that config has been initialized by auto-disovery and then it will try initialize it second time during extending class in the main config file.

najdanovicivan commented 3 years ago

You cannot have same config class with same name in multiple modules try renaming \SharedNamespace\Config\Filters to BaseFilters

But again you can only use class Filters in a single module. What about naming the extended configs as

class {ModuleName}Filter

najdanovicivan commented 3 years ago

@paulbalandan what about adding namespace for config like we have for language files

config($class . '.' . $namespace);

But again this will have to be handled in .env file which further complicates things

jbotka commented 3 years ago

@paulbalandan what about adding namespace for config like we have for language files

config($class . '.' . $namespace);

But again this will have to be handled in .env file which further complicates things

Hi, I simply disabled auto-load feature for everything and handling loading manually :) Works fine...

MGatner commented 3 years ago

This is because Filter alias discovery uses the same pattern: {namespace}/Config/Filters.php

Basically, Filters.php outside of App has conflicting use. The "quick fix" to use it as a Config class is to disable filter module discovery (app/Config/Modules.php), or to implement a Registrar for the Filters Config changes you need. But this will need some resolution in the framework itself.

MGatner commented 3 years ago

I've looked into this more and decided that, while the name conflict is not ideal it is not actually a problem. If you need to "extend" an App config you should be using Registrars, not interposing module config classes. Filters is really the only place this would cause issues though, but simply renaming the class (as others noted above) is an easy workaround.