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

Bug: Double initializing of class #3855

Closed
jbotka opened this issue Nov 4, 2020 · 9 comments
Closed

Bug: Double initializing of class #3855

jbotka opened this issue Nov 4, 2020 · 9 comments
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@jbotka
Copy link
Contributor

jbotka commented Nov 4, 2020

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

  • OS: Windows 8.1 x64
  • Web server Apache
  • PHP version 7.4.11
@jbotka jbotka added the bug Verified issues on the current code behavior or pull requests that will fix them label Nov 4, 2020
@paulbalandan
Copy link
Member

Can you describe more of your issue?

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

@jbotka
Copy link
Contributor Author

jbotka commented Nov 5, 2020

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
Copy link
Member

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
Copy link
Contributor Author

jbotka commented Nov 5, 2020

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
Copy link
Contributor

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
Copy link
Contributor

najdanovicivan commented Feb 13, 2021

@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
Copy link
Contributor Author

jbotka commented Feb 14, 2021

@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
Copy link
Member

MGatner commented May 8, 2021

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
Copy link
Member

MGatner commented May 9, 2021

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.

@MGatner MGatner closed this as completed May 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

No branches or pull requests

4 participants