Skip to content

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

Merged
michalsn merged 1 commit into
codeigniter4: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 into
codeigniter4: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.

@michalsn michalsn left a comment

Copy link
Copy Markdown
Member

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

kenjis commented Jan 29, 2022

Copy link
Copy Markdown
Member

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

iRedds commented Jan 29, 2022

Copy link
Copy Markdown
Collaborator

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

@kenjis

kenjis commented Jan 29, 2022

Copy link
Copy Markdown
Member

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

@kenjis

kenjis commented Jan 29, 2022

Copy link
Copy Markdown
Member

@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

iRedds commented Jan 29, 2022

Copy link
Copy Markdown
Collaborator

@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

kenjis commented Jan 29, 2022

Copy link
Copy Markdown
Member

@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

iRedds commented Jan 29, 2022

Copy link
Copy Markdown
Collaborator

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

@kenjis

kenjis commented Jan 29, 2022

Copy link
Copy Markdown
Member

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

kenjis commented Jan 29, 2022

Copy link
Copy Markdown
Member

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

kenjis commented Jan 30, 2022

Copy link
Copy Markdown
Member

@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