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

Deprecate transparent extension #3

Open
lenton opened this issue Jan 23, 2015 · 3 comments
Open

Deprecate transparent extension #3

lenton opened this issue Jan 23, 2015 · 3 comments
Milestone

Comments

@lenton
Copy link
Contributor

lenton commented Jan 23, 2015

Transparent extension via the cascading filesystem has become the bane of Kohana's existence. On the surface transparent extension seems like a great idea, that is until you dig a little deeper and realise all of its imperfections. Transparent extension hinders the development of Kohana in that it forces non-modern, annoying, and strange conventions.

Here are some flaws of transparent extension (you may know of more):

  • Forces us to create an empty child class for all classes - normally in the format of Kohana_Foo -> Foo. Without an empty child class users can't extend specific methods, they would have to duplicate the whole class. It becomes even messier when a module extends something from the core as it has to create another parent and child class: Module_Foo -> Foo.
  • When referencing a class's own property or method you're forced to use the explicit class name Foo::$bar instead of self::$bar. The reason is explained nicely here by @evanpurkhiser.
  • Two different modules can't extend the same class as one of them will be ignored.
  • IDEs don't know when classes have been extended via the cascading filesystem and as a result cannot provide correct information to you when developing.

I believe that transparent extension was built at a time when PHP wasn't so good at it. Let's rid ourselves of this outdated, hacky implementation and get back to writing clean PHP using the proper method of extension. Deprecating it now would give users (and us developers) ample warning to start eradicating the use of transparent extension before the next major release.

Replacement

Instead of using the CFS autoloaders it would be recommended to register modules with the composer PSR-0 autoloader. Over the period of the next minor releases leading up the to the major, inversion of control should be applied to all existing classes. Users will then be able to inject their own or extended implementations instead of using transparent extension. Dependency injection (if applied to all classes) would offer the same extensive ability as the CFS provided.

Targeted Version

Deprecation: 0.1 (Kohana 3.4)
Removal: 1.0 (Kohana 4.0)

Implementation

Deprecating transparent extension is as simple as adding @deprecated tags to the doc blocks of the autoloader classes, abstract class, and interface located in this package.

@acoulton
Copy link
Member

👍 for deprecation.

Couple of thoughts on removal:

  • I'm not sure how DI will work for static method calls (which are very common). We can fix that in the project by inverting control and using instance methods, but if end users have customised Arr::get for example I'm not sure how they can easily replace all the Arr::get calls in their project?
  • Worth starting to think about whether we can do anything to help people upgrade. Perhaps a tool that detects any transparent extension already in use in their projects and helps to rename the class/add a DI mapping/update references to the class?
  • Since removal is going to involve renaming classes and fixing extension classes, might make sense to namespace our classes at the same time, so that people can do them together.

This doesn't technically fix the "two modules can't extend same class" issue - for that, we need to make all the classes much smaller so that people can just extend/replace the specific bits of logic they need.

Also I think the self issue maybe came before we dropped PHP 5.2 support - it is possible now to use late static binding eg static::$bar to refer to whatever the extended method/constant/property is.

@acoulton
Copy link
Member

This might be a really bad idea, but what about keeping the current Kohana classes and static methods (maybe in a separate "bridge" / "bc" package) as a facade over new dependency-injection based code.

Something like:

class Arr {
  public static function get($key, $default) {
    $instance = ServiceContainer::getInstance()->get('Arr');
    // $instance would by default be an instance of say Kohana\Util\Arr which would have all the 
    // logic migrated from Kohana_Arr
    return $instance->get($key, $default);
  }
}

Obviously that would have downsides:

  • It's a bit yuck...
  • Would require a singleton global service container
  • This bridge package would have to depend on a service container (probably an interface), though the individual packages still wouldn't need to know anything about it.

But it would mean that end-user code that does Arr::get will continue to work, and all they'd have to do would be to rename their custom Arr class and override the service container mapping for it - which could usually be done automatically fairly easily.

Most people could therefore probably upgrade very quickly without major rewriting, and then gradually work towards inverting the dependencies in their own code....

Thoughts?

@lenton
Copy link
Contributor Author

lenton commented Jan 23, 2015

I'm not so keen on the idea of a service container, I think it makes things more complicated and feels like its pussyfooting around actually upgrading applications. Trouble with a bridge is that it just prolongs applications from being upgraded until we force them by removing/unsupporting it. If users need time to make the transition then they can quite as easily create a new git branch.

If we have a few more minor releases before 4.0 which primarily focus on removing static methods and allowing injection in a BC manner then users can take it step by step with us.

@lenton lenton modified the milestones: 3.4.0, 1.0.0 Feb 2, 2015
@lenton lenton modified the milestones: 0.1.0, 1.0.0 Feb 2, 2015
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

No branches or pull requests

2 participants