kohana / core

Core system classes from Kohana
http://kohanaframework.org
634 stars 327 forks source link

Namespaces for controllers modules and helpers #571

Open rjd22 opened 9 years ago

rjd22 commented 9 years ago

We should start thinking about implementing namespaces in Kohana and it's modules but also decide on a vendor / prefix.

Like \Kohana\<module>\<class>

Also enable users to namespace their own application. And point the routes to the namespace.

What do you guy's think? (Typed this on my mobile so keeping it short)

rjd22 commented 9 years ago

@acoulton I think we should not make a upgrade path available. There is a lot of stuff in Kohana we might want to clean up. Normally if you go from 3.x to 4.x you won't be able to upgrade easily.

While there are quite some good frameworks out there like symfony, my opinion is that they force standards on you that you might not want, like twig or doctrine. My vision is to make kohana take care of the stuff you don't want to deal with like routing or request handling but will not force a standard on you, and enable you to switch around libraries.

enov commented 9 years ago

:+1: for upgrade wherever possible.

they do force standards in you you might not want like twig or doctrine.

Neither Twig, nor Doctrine is forced in Symfony.

shadowhand commented 9 years ago

I think we should not make a upgrade path available.

Then why bother chasing Laravel, Symfony, and the other modern frameworks? Kohana doesn't have the manpower or community support to compete with them. The only way any of this makes sense is if Kohana doesn't break BC.

rjd22 commented 9 years ago

@shadowhand wouldn't it be extremely hard to be backwards compatible and still implement name spacing and the improvements we want?

acoulton commented 9 years ago

@rjd22 symfony is more flexible than you think, but if you want something lighter there are smaller alternatives to "take care of the stuff you don't want to deal with" - for eg orno/route. I don't see the point in trying to reinvent that.

If anything, I'd support a 4.0 where we delete pretty much all the code except the class names and method signatures and just proxy them all to better third-party libraries, over one with a totally different API that fixes solved problems.

shadowhand commented 9 years ago

@rjd22 probably, but again... what's the point if BC is not maintained?

I agree 100% with @acoulton here.

acoulton commented 9 years ago

There's absolutely no reason namespacing has to be a big deal for BC. All it needs is some namespace and use declarations at the tops of files (which could be scripted/generated) and if we were feeling kind an optional package that provides aliases in the root namespace so end users don't even have to use the namespaces if they don't want.

Dropping transparent extension is trickier, but in no way impossible.

enov commented 9 years ago

we delete pretty much all the code except the class names and method signatures and just proxy them all to better third-party libraries

So it's like using other libraries code, but with Kohana API... Is that even possible?

zombor commented 9 years ago

What's the point? I'm not sure why people are holding on here.

enov commented 9 years ago

What's the point? I'm not sure why people are holding on here.

Maybe we have applications written on top of Kohana?

zombor commented 9 years ago

And they still work. If you aren't proposing 100% BC, what's the point? You'll have to change all your code anyway.

lenton commented 9 years ago

The only way any of this makes sense is if Kohana doesn't break BC.

+1 The whole point of continuing with Kohana instead of abandoning it and contributing to Ohanzee was to create a smooth transition path for users to upgrade.

wouldn't it be extremely hard to be backwards compatible and still implement name spacing and the improvements we want?

If 4.0 still supports Kohana modules then it won't be as bad, I'm working on that now.

enov commented 9 years ago

I am for an upgrade path, @zombor. Yes, if I need to change all my code, what's the point..

zombor commented 9 years ago

You can't have a "modern" framework without breaking BC in a major way, imo.

enov commented 9 years ago

I am not expecting a fully modern framework in 4.0, just step by step.

acoulton commented 9 years ago

we delete pretty much all the code except the class names and method signatures and just proxy them all to better third-party libraries

So it's like using other libraries code, but with Kohana API... Is that even possible?

Possible, probably pointless in many cases. Meant it as an extreme example, but underlining that I think it's only worth updating Kohana to make it easier for people who have existing applications to support/gradually disentangle them.

There are two possible future users of Kohana:

None of those people are going to use 4.0 - no matter how good it is - if it's totally different because there are existing, better, supported, active projects with communities around them already. Anyone who thinks we'll suddenly attract new people that aren't in those two groups is chasing rainbows.

acoulton commented 9 years ago

I am not expecting a fully modern framework in 4.0, just step by step.

I'm not expecting a modern framework at all, just one that doesn't get in the way of me writing/migrating to a modern application. For me that mostly just means:

And that's about it.

zombor commented 9 years ago

If you write your application abstracted away from any framework, you satisfy all these requirements right now. The framework is a detail.

acoulton commented 9 years ago

@zombor I do, and I'm moving the older projects that way as I can.

But 3.3 does get in the way of that at times, not least the fact that controllers have to be in the global namespace as is all the kohana stuff...

rjd22 commented 9 years ago

BC compatibility seems still important so let's aim for that.

I would like to hear options on how to achieve it while supporting namespaces, PSR and removing the transparent extension.

IMO: the most hard part will be implementing PSR 1 and 2

enov commented 9 years ago

I think there is a tool for PSR-1 and 2: http://cs.sensiolabs.org/

acoulton commented 9 years ago

Why bother with PSR-1 and 2? Don't we just risk introducing bugs that weren't there before? Do we seriously ask end-users to search and replace everywhere they call a snake_cased Kohana method just so we can rename them? And what about things like controllers - are we going to change the router to look for actionSomething instead of action_something and then force everyone to change all of that too?

I really don't see that there's any benefit to changing coding standard in the existing code if the focus of the project is BC and legacy support.

If we're going to ask people to change their code for 4.0, we should respect their time and only make them do it for things that make a functional difference eg namespaces.

rjd22 commented 9 years ago

@acoulton I think you said that you were working on the namespace support for the kohana routing. I'm in need of this so I'm thinking of working on it myself. Are you already working on this? And how we're you planning to implement it?

acoulton commented 9 years ago

@rjd22 sorry, no not so far. I think I just said I didn't think it would be hard. I think you'd just have to modify Kohana_Request to accept an extra namespace parameter from the route definition and store it as a request property same as eg directory. Then Kohana_Request_Client_Internal would need to be modified to compile the fully qualified class name.

I'd suggest that if a namespace is defined, we shouldn't add a Controller_ prefix - and possibly should ignore the directory too. Maybe Route::set should throw if you try to define both namespace and directory on the same route, to avoid harder-to-trace problems later.

So eg a route like:

Route::set('default', '(<controller>(/<action>(/<id>)))')
     ->defaults(array(
                     'namespace' => '\My\Application\Controller',
                     'controller' => 'welcome',
                     'action'     => 'index',                      
                ));

would map to \My\Application\Controller\Welcome.

For bonus points I guess we could define controller_prefix and controller_suffix as route params so that you can still use <controller> as a dynamic URL parameter but be able to for eg call your class \My\Application\Controller\WelcomeController or whatever suits you. I tend to end up with a route per controller anyway, so it doesn't much matter to me.

I don't see that adding any BC issues except for the rare possibility that someone is using namespace as an actual request parameter name. In that case they'd get a 404 from those URLs on first upgrade because their namespace parameter is unlikely to map to an existing class.

rjd22 commented 9 years ago

@acoulton looks good to me. I will see if I can make time this weekend. This should be able to go into 3.4

acoulton commented 9 years ago

@rjd22 great, thanks :)

rakucmr commented 9 years ago

@rjd22 Maybe this can help you https://github.com/kohana-ns

loonies commented 9 years ago

Why don't we just drop namespace part in the route definition? Define a route and map it to a FQCN and action. This way you can use arbitrary class as a controller. The only requirement would be that a class extends base controller class where request/response are injected.

rjd22 commented 9 years ago

@loonies for the sake of discussion can you give some code examples?

acoulton commented 9 years ago

@rjd22 I presume @loonies means just:

\\ Maps to \My\Application\Controller\Welcome::action_index()
Route::set('welcome', '(welcome(/<action>(/<id>)))')
     ->defaults(array(
                     'controller' => '\My\Application\Controller\Welcome',
                     'action'     => 'index',                      
                ));

Issue with that is it would require a separate route for each controller, since you probably don't want an FQCN in your URLS. I mostly do that anyway, but I think a lot of users don't and it wouldn't be BC with existing routes.

A halfway house might be to say eg:

if (substr($params['controller'], 0, 1) === '\\')
  $controller = $params['controller'];
elseif ($params['namespace'])
  $controller = $params['namespace'].'\\'.$params['controller']; // could apply optional prefix/suffix here
elseif ($params['directory'])
  $controller = $params['directory'].'_Controller_'.$params['controller'];
else
  $controller = 'Controller_'.$params['controller'];

Which would be fully BC, but would support new code providing either FQCN or namespace + class name.

loonies commented 9 years ago

Yes, that's what I meant.

@acoulton: Good point with using controller as a param issue.

At least I would like that we drop the "Controller_" prefix or have the ability to use arbitrary class name as a controller.

localheinz commented 9 years ago

This thread is tl;dr, so:

It's 2015, there are a lot of people using PHP5.5 and even PHP5.6 in production already. Not moving on and leveraging features of the PHP version required in composer.json will eventually mean to be left behind by the community, which may or may not move on to more up-to-date frameworks.

So, totally :+1: for using actual instead of pseudo-namespaces.