CodeIgniter4 icon indicating copy to clipboard operation
CodeIgniter4 copied to clipboard

Local Namespace

Open MGatner opened this issue 4 years ago • 18 comments

Concept Defines a third namespace sector for downstream application modification, Local.

Description This is something I am starting to incorporate into my open-source applications. The problem this solves is something like this:

  • User finds a great open-source app, clones repo
  • User changes the color of a menu, code is now deviant from upstream
  • Developer releases updates, user must now go through "Upgrading from xx to yy" steps every time

By defining a Local namespace that resides outside of the repository it allows for modifications while leaving the application code untouched. Think of this as the equivalent of applications using code in System without modifying it.

This PR is the initial concept to feel out the maintainers and community before extending the concept deeper into the system code, User Guide, tests.

Checklist:

  • [X] Securely signed commits
  • [X] Component(s) with PHPdocs
  • [ ] Unit testing, with >80% coverage
  • [ ] User guide updated
  • [X] Conforms to style guide

MGatner avatar Jan 07 '21 16:01 MGatner

  1. What is a OSS app?
  2. Can you give a specific example?
  3. If the application has changed and the changes made by the user are not saved. This is the problem of the application developer, not the user.

iRedds avatar Jan 07 '21 18:01 iRedds

Sorry to be unclear, hope this helps. OSS = Open Source Software (edited original)

Simplistic example, I create a new CI4 project and create a Model, Views, and Controller for basic management of Users. I brand it, add a license and publish it on GitHub. You think its great, clone it and serve it from your Docker instance. You decide you want a new route to return a JSON representation of a User, so you add the route in app/Config/Routes.php and create a new json($userId) method in app/Controllers/Users.php: good to go. But oops, I have updated the original project with a new route also, for Users as a CSV (add the route in app/Config/Routes.php, create a new csv($userId) method in app/Controllers/Users.php). I create a new release, and now downstream you have to deal with merging code, or following an update guide, or sticking with your version.

Obviously the above happens all the time, but what I'm interested in are tools either to assist with the process or mitigate the situations where the conflict happens. With this Local namespace concept, instead of editing the application code you (as the consumer of my app) created local/src/Config/Routes.php and local/src/Controllers/Users.php with the new route and method respectively and never had to touch the distributed code. When the upstream release comes there is nothing for you to change since your project is still in sync with the original.

Does that help explain it?

MGatner avatar Jan 07 '21 19:01 MGatner

I don't think I understand the concept completely, yet.

Is the OSS (which means Open Source Software, I think) in our case CodeIgniter itself, a ThirdParty Module or something totally different?

It would be nice if you could provide an example with CodeIgniter (whatever use case) :-)

sfadschm avatar Jan 07 '21 19:01 sfadschm

@sfadschm Thanks for the reply! I think we crossed paths, see my reply above.

MGatner avatar Jan 07 '21 19:01 MGatner

Jup, 42 seconds :-D

Thanks for the details, that makes it clear I think.

Seems like a really useful addition to me. I think publishing whole apps instead of modules might/should be a thing and this will definitely ease the process.

That's a +1 from my side.

sfadschm avatar Jan 07 '21 19:01 sfadschm

@MGatner I like this but I'd suggest adding an additional constant for LOCAL_NAMESPACE to be able to change the name if needed.

I'd also add local directory in .gitignore so that it makes it easier for future framework development. I usually have some test controllers/models/entities when I'm doing framework changes so this one can be used to keep those files.

najdanovicivan avatar Jan 07 '21 19:01 najdanovicivan

I'm not sure if such functionality will make a lot of sense in real life cases. Not that the idea is wrong, but more like... changes made by developers to their projects are hard to predict and trying to handle them in this way (false sense of safety?) may be dangeous.

I will stick to your example with user management. Let's say that we added something to the project's configuration file (maybe email address from which emails are sent during user registration). A new version comes out with new options in the config file, but we have our own version in "Locals", so after udate the whole component stops working because some new required config variables are missing.

I don't thing that our own version of files in "Locals" can guarantee stability of the package we're using. If we're changing something in the the local version of package, then manual verification is always required during update.

If project we're using have no build-in way to make some overrides in a not invasive manner, then there are small chances that we will be able to accomplist it in a safe way.

I'm not saying no to this, but I have mixed feelings so far.

michalsn avatar Jan 07 '21 19:01 michalsn

@MGatner thanks for the explanations.

I think I understand what you mean. But it seems to me that you have chosen the wrong way.

You want to hard-code functionality that not everyone needs. Instead, it seems to me it would be better to add a tool that will help replace/extend part of the core of the framework.

Example based on your explanation

Let's say we have a application with the following structure 2021-01-08_05-01-22 The location is not really important. The paths can be written in composer.json

Next, we use the replace/extend tool. To whom we say that instead of the \CodeIgniter\CodeIgniter class, should use the \MGatner\GreatOSS\Core\CodeIgniter class.

In class \MGatner\GreatOSS\Core\CodeIgniter, we first load routes from vendor/mgatner/great-oss/Config/Routes.php and then from APPPATH . Config/Routes.php

As a result, little-used functionality is not added to the framework. OSS has its own directory. The end developer will still use the default application directory. And everyone else gets the ability to expand the core of the framework.

iRedds avatar Jan 07 '21 21:01 iRedds

an additional constant for LOCAL_NAMESPACE

@najdanovicivan Good idea! Also this will definitely be in .gitignore - still lots to implement, I just wanted to float the idea first.

MGatner avatar Jan 15 '21 01:01 MGatner

@michalsn Your logic is sound but I think it misses the point. This could be a tool for the end user, but really it is a tool for the penultimate user, the app developer. An analogy...

  • A PHP Language Architect must always be aware of changes to the language, because anyone using PHP will be affected by breaking changes; this requires changes to the language to be made in such a way that a Framework Developer (you & me) can predictably a) take advantage of new features, and b) not have everything break with point releases.
  • Likewise, a Framework Developer must be mindful of an App Developer who uses our framework, and make sure new features and changes don't break their apps. Normally we would stop there, and any App Developer who publishes an app would expect a Local Developer to use their app and either a) never modify it, or b) hack it up but read through changelogs to implement changes every time they happen.
  • This PR suggests we allow (not require) Local Developer to have the same opportunity from App Developer that our App Developer gets from us. An app supporting the Local namespace would follow semantic versioning such that minor releases would be compatible with anything that might be available to Local.
  • Just like we cannot enforce expected division of app/ and system/, an app cannot enforce expected use of local/ and app/ - but the tool is there for developers to work with an publish docs about.

I will add about this analogy that the process I describe for the App Developer is already what I do personally, and what I would consider "good practice".

MGatner avatar Jan 15 '21 01:01 MGatner

@iRedds Thanks for your comment. I believe you've just describe modules/libraries which, while very important to the future of the framework, already have good representation and are not really being addressed here. If I have misunderstood please correct me.

MGatner avatar Jan 15 '21 01:01 MGatner

@MGatner Perhaps my example looks like a module. But I tried to show interaction. The class replacement tool will help in both your case and other situations. For example, replacing part of the database driver.

It seems to me that what you are proposing is a private case. And if I were in your place, I would fork the framework and make the appropriate changes necessary to work correctly with my application.

I think it would be better to conduct a survey among the users of the framework. Anyway, this is just my opinion. Compared to what you are doing for the framework, I am nothing.

iRedds avatar Jan 15 '21 04:01 iRedds

@iRedds All opinions are worth hearing! In this case I'm not too worried about "extra unused functionality" because it amounts to one config value and a single is_file() check; we could even leave it out of app/Config/Autoload.php by default and just note the option in the User Guide.

Maybe I am not understanding the "class replacement tool" part of it, on how this is different from a module. We brought up class aliases as part of the framework but decided against them - maybe you're suggesting the same thing just implemented per project?

I should add that I am already implementing this myself in projects, and will do so if we decided this does not belong as part of the framework. Right now I adding the namespace to Autoload config and then checking for the Composer autoload file in Common.php - see for example this project commit. It works fine, but I think we could expand the offering if it were part of the core - things like discovery priority over App.

MGatner avatar Jan 15 '21 14:01 MGatner

@MGatner Hmm... maybe I understood it in the wrong way... Others seem to be enthusiastic about it, so I guess it can be a useful feature.

michalsn avatar Jan 16 '21 18:01 michalsn

@MGatner I have an idea on how to handle this. Why hardcode composer loading to the local namespace only. Why don't we add additional property in Config/Autoload containing string array of namespace for which composer should be loaded

public $psr4 = [
		APP_NAMESPACE => APPPATH, // For custom app namespace
		'Config'      => APPPATH . 'Config',
		'Local'       => ROOTPATH . 'local/src',
];

public $composerPsr4 = [
   'Local'
] ;

So as we have Local in $composerPsr4 the framework will require `ROOTPATH . 'local/src/vendor/autoload.php',

So in the end Local namespace does not even have to be used or defined anywhere by default. And with $composerPsr4 multiple directories can be used.

najdanovicivan avatar Jan 17 '21 21:01 najdanovicivan

@najdanovicivan I see some merit to expanding the Composer autoload. If we're doing that it probably would make sense to make them actual file paths, instead of namespaces which don't technically have autoload - so in your example above it would be:

public $psr4 = [
		APP_NAMESPACE => APPPATH, // For custom app namespace
		'Config'      => APPPATH . 'Config',
		'Local'       => ROOTPATH . 'local/src',
];

public $composerPsr4 = [
   ROOTPATH . 'local/src/vendor/autoload.php',
] ;

Local namespace does not even have to be used or defined anywhere by default

The Composer autoloading was one small extension of the larger goal: differentiating a preset modular namespace and giving it priority over App. I think the extent of this will become more clear when I implement the rest of this PR - I think I've had enough positive reinforcement to continue with it.

MGatner avatar Jan 18 '21 02:01 MGatner

@MGatner any movement on this?

My personal opinion is that it adds some unnecessary complication to the app, but if I'm in the minority that's cool. Might be worth a discussion on the forums if we're still up in the air?

lonnieezell avatar Jan 05 '22 04:01 lonnieezell

I use this in my own open-source published apps. I am still baffled what people do without it, and believe it's an important feature for supporting developers who want to make "distributable apps".

Maybe the thing to do would be to finish this PR in earnest and ask for updated reviews and opinions?

MGatner avatar Jan 05 '22 13:01 MGatner

Well I haven't touched this. I still believe in it but it doesn't seem a priority to anyone else and it only takes me about 30s to set it up in my local apps so I'm going to consider this abandoned.

MGatner avatar Sep 16 '22 10:09 MGatner