Skip to content

add app and app/Config to autoload composer.json for app starter#3423

Merged
michalsn merged 1 commit intocodeigniter4:developfrom
samsonasik:app-config-composer-starter
Aug 3, 2020
Merged

add app and app/Config to autoload composer.json for app starter#3423
michalsn merged 1 commit intocodeigniter4:developfrom
samsonasik:app-config-composer-starter

Conversation

@samsonasik
Copy link
Copy Markdown
Member

The starter app is mostly installed via composer create-project. I think it is better to use composer autoload for it.

Checklist:

  • Securely signed commits

@paulbalandan
Copy link
Copy Markdown
Member

I think we should exclude App\ from here as we support custom namespace via APP_NAMESPACE.

@samsonasik
Copy link
Copy Markdown
Member Author

That change may less happen, also, composer autoload doesn't break codeigniter 4 autoloader, that will be a fallback namespace.

Copy link
Copy Markdown
Member

@michalsn michalsn left a comment

Choose a reason for hiding this comment

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

I can't think of any disadvantages of this solution.

@michalsn michalsn merged commit e2f9a1d into codeigniter4:develop Aug 3, 2020
@samsonasik samsonasik deleted the app-config-composer-starter branch August 3, 2020 13:43
@kenjis
Copy link
Copy Markdown
Member

kenjis commented Jan 29, 2022

We cannot change app folder name. Because the composer's path overwrite the config in Config\Autoload.
https://forum.codeigniter.com/thread-81153.html

@samsonasik What's the benefit to use composer autoload?

@samsonasik
Copy link
Copy Markdown
Member Author

As far as i remember, with prioritize composer autoload when possible, it will got benefit for composer itself (eg: classmap on dump-autoload -o).

Probably in case of changing app directory, documentation need to be updated to change composer.json as well.

@iRedds
Copy link
Copy Markdown
Collaborator

iRedds commented Jan 29, 2022

It's time to abandon the internal autoloader and use only the composer.

@kenjis
Copy link
Copy Markdown
Member

kenjis commented Jan 29, 2022

@samsonasik Thank you. I'm not sure that composer's autoloader has priority. I'll check.

@kenjis
Copy link
Copy Markdown
Member

kenjis commented Jan 29, 2022

@iRedds Do you mean the zip installation will be discontinued?

@lonnieezell
Copy link
Copy Markdown
Member

One core tenet of CI has always been the ability to download a zip and not worry about anything. From my experience on the forums since v4 has been released this is still the case for a large chunk of users.

We must always remember the broader user-base and CodeIgniter's history. While we don't want to be completely beholden to old ways, we do need to pay attention to the users, and not alienate a broad group of them for no discernible gain.

@paulbalandan
Copy link
Copy Markdown
Member

This could have been avoided if we did not have the autoload.psr4 section. I tried with codeigniter4/codeigniter4 renaming app/ to myapp/, and the only thing I need to change is the FCPATH in spark and public/index.php.

@iRedds
Copy link
Copy Markdown
Collaborator

iRedds commented Jan 29, 2022

@kenjis Do you need a zip installation?
@lonnieezell The CI history says use an array for configuration files. =)
Do you have zip download data to say that this is the majority?
For example, Composer generated 37,735 installs in the last 30 days, of which 1,311 were made today alone.
https://packagist.org/packages/codeigniter4/framework/stats#major/4

@kenjis
Copy link
Copy Markdown
Member

kenjis commented Jan 29, 2022

@iRedds Personally I never use zip installation, but I don't say drop it soon.
CI is the last major framework that can be installed without Composer.
I think drop of the Zip installation should be carefully considered.

I see users using Zip who report GitHub issues sometime.

@iRedds
Copy link
Copy Markdown
Collaborator

iRedds commented Jan 29, 2022

@kenjis Composer.phar and the installed framework can be added to the zip.

@kenjis
Copy link
Copy Markdown
Member

kenjis commented Jan 29, 2022

The path is from Composer, but
it seems CI's autoloader loads App\Controllers\Home. CI 4.1.8 appstarter.

Screenshot 2022-01-29 18 33 58

@kenjis
Copy link
Copy Markdown
Member

kenjis commented Jan 29, 2022

No, @samsonasik is correct.
If I run composer dump-autoload -o, 'App\\Controllers\\Home' is included in Composer classmap file,
and CI4 autoloader uses it.

@kenjis
Copy link
Copy Markdown
Member

kenjis commented Jan 30, 2022

@iRedds

Composer.phar and the installed framework can be added to the zip.

I don't think it is good idea.
It forces users to use Composer without permission.

kenjis added a commit to kenjis/CodeIgniter4 that referenced this pull request Mar 23, 2022
…ser.json

Revert codeigniter4#3423

Problems:
- Cannot change `app` folder name. Because the composer's path overwrite the config in Config\Autoload.
- Causes error when defining new namespace under app/. See codeigniter4#5818
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.

6 participants