getkirby / ideas

This is the backlog of ideas and feature requests from the last two years. Use our new feedback platform to post your new ideas or vote on existing ideas.
https://feedback.getkirby.com
20 stars 0 forks source link

Change the starterkit/plainkit index.php file #371

Open hdodov opened 5 years ago

hdodov commented 5 years ago

Currently, the plainkit index.php file looks like this:

require 'kirby/bootstrap.php';
echo (new Kirby)->render();

According to the source, the kirby() helper function calls App::instance() which returns the current active Kirby instance or creates a new one. Why not use that instead? It basically does the same thing except it has the added benefit of an already created instance. This can be useful if you've installed a utility package (I'm working on one) that creates a Kirby instance with some configuration. For example:

  1. The bootstrap.php includes the Composer autoloader
  2. Composer loads the utility package
  3. The utility package creates new Kirby($someConfig)
  4. kirby() returns the utility package Kirby instance

On the other hand, if you use none of that, you have the same behavior as you do now.


So my suggestion is to change index.php to this:

require 'kirby/bootstrap.php';
echo kirby()->render();

It looks cleaner and less confusing too 🙂

lukasbestle commented 5 years ago

What I like about the change is that looks cleaner. You are right about that.

I'm not so sure about the utility package approach however. You can definitely do it that way for your own sites, but that package is probably not suitable to be published because relying on the state of singleton objects isn't reproducible. If any other installed dependency creates an instance as well, it will break.

Anyway: I like the idea of changing the index.php, but for different reasons. :)

hdodov commented 5 years ago

If any other installed dependency creates an instance as well, it will break.

I don't think anything would break. Since instances overwrite each other, you'd simply be using the latest one, i.e. all overwritten ones will just be unused.

Those utility packages could be implemented in a way that handles cases where there is no instance and where there is. Although I agree this can be fragile, it's a problem only when there is third party software in play, and that software could warn about those potential issues.