[do not merge] Change the default directory structure and use new features of 3.3 #1034

dunglas · 2016-12-16T10:58:20Z

This PR leverages is part of the 0 config initiative. It leverages the numerous improvements done in 3.3 and ActionBundle:

The app is now bundle less: all files are now directly in the srcdirectory, no AppBundle anymore
Any object can now be injected as a service in controllers (using constructor, getter or setter injection) without having to define it as a service in services.yml thanks to autowiring
Common controller's proxy methods are injected using a trait (ControllerTrait)
Getter injection is used in the controller (lazyness everywhere)

mickaelandrieu · 2017-01-10T10:05:50Z

app/config/services.yml

@@ -4,6 +4,5 @@ parameters:
 #    parameter_name: value

 services:
-#    service_name:
-#        class: AppBundle\Directory\ClassName
+#    'My\ClassName':


One question: in your tests, how can you replace this service with another one (to use stubs for instance)?

like any other service: by defining the class name explicitly:

services: 'My\ClassName': class: 'My\TestClassName'

Yeah, this is what I'm afraid of: you're forced to mix both syntaxes :/

what both syntaxes ? We don't have 2 syntaxes. We just made class default to the id when it is omitted. The id of your service is just My\ClassName in this case.
You are free to keep using other ids if you prefer btw. Services will not be forced to use a class name as id.

Services will not be forced to use a class name as id.

Ok, I'm very afraid to be forced to use it, please keep it possible even for Sf4 please :)

Forcing to use the class name does not make sense as you would then not be able to register more than one service for a class.

Keep in mind that this new "feature" si optional, don't use it if you don't like it. Anyway, IMHO, if you need to change the class of a service for testing, you are doing something wrong.

Thanks @fabpot.
Agree with you at some point, it could be useful to set new services in "test" environement when you want a "null" strategy like using in carsonbot for instance (https://github.com/carsonbot/carsonbot/blob/master/app/config/services_test.yml#L7). Not the topic, but if it's wrong I'd like to know why :)

fabpot · 2017-01-10T20:27:10Z

app/config/routing.yml

-app:
-    resource: "@AppBundle/Controller/"
+action:
+    resource: "../../src/Controller/"


This works by chance :) see symfony/symfony#21231

dunglas · 2017-01-11T07:35:45Z

Now includes updated versions of:

It's ready to test.

lunika · 2017-01-13T10:51:16Z

I really like the idea of Bundleless symfony applications but with this structure how we should register some features like :

compilerPass, I usually put them in AppBundle::build method
custom configuration : a Configuration class is put in DependencInjection directroy and used in the AppExtension class where the config is computed and injected in the container (for custom parameters for example)

thanks

stof · 2017-01-13T11:06:07Z

custom configuration : a Configuration class is put in DependencInjection directroy and used in the AppExtension class where the config is computed and injected in the container (for custom parameters for example)

if you need a semantic configuration, you will need a bundle (well, technically, you need a DI extension, but adding one without a bundle becomes painful even though it is possible, as non-bundle extensions are called only when they have some config explicitly)

compilerPass, I usually put them in AppBundle::build method

you can override Kernel::prepareContainer to register additional compiler passes (don't forget to start with a parent call, otherwise you will break any bundle-provided DI features)

chalasr · 2017-01-14T22:33:21Z

I personally agree with all the goals of 3.3 (it took me some time to be ok with some of them though, but here we are).
However, I do think bundles are a great feature that is imo not something hard to understand when starting to use symfony (as opposed to the container and all related stuff), even more, it gives an idea of what is separation of concerns and a good opportunity to start applying it for then, when becoming pretty comfortable with symfony, start moving stuff outside of bundles.
Nobody forces you to use bundles as well as nobody forces you to use autowiring, but you can at least find an example of this great concept and start playing with it as you wish thanks to the AppBundle. Manipulating bundles in its own app also makes easy to understand how to manipulate and write 3rd party ones, thus reuse instead of reinvent.
So, do we really want to remove the concept of bundles from the default structure?

(It's just a question, do not see any offence, I do love bundle-less apps, but only after having used bundles for years since these ones given me a structured way to organize my code and, I fairly trust it, helped to make me a better developer)

pocky · 2017-01-15T14:40:03Z

Hello folks,

I love this PR but I also have a question related to my freelance position with lot of legacy-to-(symfony|microkernel|silex) projects. If there is no more AppBundle and the "best practices" encourage us to add our configuration, views, etc in app, why you don’t simply move src to app/src ?

There is (at least) three advantages to do this for DX and/or project :

I know a lot of people hate this but (even if I don’t like it too), renaming the app folder and add another one for multi-app (ok you will have problems with ScriptHandler but multi-app is not standard so...)
For monolithic projects, add your frontend application (vue.js for example but it works with a lot of things) aside to your backend application
If an application is growing up : cut/paste/done.

And for @chalasr because his question is a true one, rename the actual srcto bundle could be an answer.

And again, I love this PR :D

dunglas · 2017-01-15T15:32:38Z

The goal of this PR is to experiment with automatic service registration and bundle less apps. However the final target is Symfony Flex (it's why this PR has the [do not merge] flag). Let's see what it looks like first.

Update dependencies

dunglas · 2017-02-22T15:15:22Z

I've updated this PR to use new features of 3.3 instead of ActionBundle:

no more AppBundle
controllers and commands are automatically discovered and registered as services
controllers and commands are autowired, you can typehint any class in their constructors and use it as a service without having to register it manually (tldr: to create a new service: create a class somewhere in src, typehint it in your constructor or anywhere else, it will be automatically instantiated and injected)
for newcomers and performance sensitive apps, the ControllerTrait can be used to automatically inject common services (same as the current Controller class) through getter injection. This feature is not mandatory (if you want to cherry-pick deps of your controller, just typehint services you need in its constructor) but is a shortcut for newcomers showing all the power of Symfony (common features).

I've also updated the directory (just a proposal):

conf/: config files
- dev/: conf overriding for the dev env
- prod/: conf overriding for the prod env
- test/: conf overriding for the test env
src/: all PHP code (including the kernel)
- no more AppBundle
- Controller/: Controllers
templates/: Twig templates (can be removed if you don't use Twig - e.g. API Platform)

Unchanged:

bin/
var/
vendor/
tests/
web/

ruudk · 2017-02-22T15:24:33Z

Nice! I have a question: Why not do config/ instead of conf/?

dunglas · 2017-02-22T15:25:48Z

@ruudk It's shorter and maybe more common, but I've nothing against config.

lyrixx · 2017-02-22T15:34:47Z

@dunglas As you are breaking everything, could we broke also:
app/Resources/views/base.html.twig → templates/base.html.twig to app/Resources/views/base.html.twig → templates/layout.html.twig (base->layout) ?

mickaelandrieu · 2017-02-22T15:36:38Z

@dunglas really interesting, this also need a MASSIVE update of docs and probably a new "Best practices" standard or all apps will become a mess ^^

Is this intended to replace the actual Symfony full edition? Or is Symfony flex a new "edition"?

dunglas · 2017-02-22T15:39:04Z

@mickaelandrieu it's just a personal experiment for now, nothing official (it's why the PR title contains [do not merge].

dunglas · 2017-02-22T15:42:35Z

@lyrixx done

joelwurtz · 2017-02-22T15:57:34Z

Where does your ressources directory will go with this structure ? (translations / assets / ...)

EDIT : Imo it should be a directory inside src/ but maybe it should be explicitely set to show the best practice about this ?

yceruto · 2017-02-22T16:06:59Z

According to the new structure (purposed) the configuration path should be

public function registerContainerConfiguration(LoaderInterface $loader)
{
    $loader->load($this->getRootDir().'/../conf/'.$this->getEnvironment().'/config.yml');
}

(i.e. /conf/dev/config.yml)

https://github.com/symfony/symfony-standard/pull/1034/files#diff-aa0dbf36ede817d01f5be6f15d867bb4R65

yceruto · 2017-02-22T16:28:08Z

src/Controller/Hello.php

+    /**
+     * @Route("/", name="homepage")
+     */
+    public function __invoke(Request $request): Response


Is mandatory to use __invoke() method here? if you are using route annotation, why not use a common method name instead? such as index for example :/

It's to propose the concept that a controller represents a single action.

Using index() would encourage to create other methods, whereas using __invoke() will encourage you to create one single route bound to this controller.

Why not recommend (again?) the use of xml/yml for routing instead of using annotations? This would seriously ~~decouple~~ encourage decoupling from the SensioFrameworkExtraBundle, and also reinforce the "single-action controller" concept proposed in ADR pattern

Why do you consider that an annotation is coupling? You can perfectly ignore it and do otherwise if you'd like to. It's plain descriptive only to me. "Use if you want". Thus no coupling at all.

Using index() would encourage to create other methods, whereas using __invoke() will encourage you to create one single route bound to this controller.

It is confusing to me, Hello is a controller (src/Controller/Hello.php), a controller has actions (in most cases), if the concept is "A controller represents a single action" it mean that I need a controller per action? so the path shouldn't be src/Action/Hello.php then? We need create a php file per action then? :/

EDIT: I know it's optional, but IMHO default concepts should follow common cases.

For Symfony, a controller is a callback. So in fact in the traditional Symfony structure the controllers are the methods not the classes, they are just badly named.

@yceruto in HttpKernel, the controller is not the class. It is always the callable.

A controller cannot have actions. A single class can contain multiple controllers. Controllers are always methods in Symfony (unless you go back to symfony 1 where the term controller had a different meaning).
Btw, when using Silex, it is very common to avoid using classes entirely and use closures instead (which cannot be done in the fullstack framework as it does not play well with caching)

@stof thanks for the explanation, to be clearer, we are defined two possible controllers here (at the same time):

'App\Controller\Hello'

'App\Controller\Hello::__invoke'

Currently we are using the second controller with the appearance to use the first one (which is confusing), so my suggestion here is to use a better method name if we use already @route() annotation, otherwise let's define the controller into some routing.yml to use the first one.

Now in Symfony it is not possible to declare this type of route as annotation but could be good to have something like this for these cases:

/** * @Route("/", name="homepage") */ class Hello { public function __invoke() { return new Response('Hello World!'); } }

Actually equal to:

homepage: path: / defaults: { _controller: src\Controller\Hello }

@yceruto Your suggestion to put the route annotation on the class for invokable controller class is very interesting, and could be the object of an issue/PR in the SensioFrameworkExtraBundle repo 😄

Pure MVC was never meant to be used on the web, ADR better describes how a web application works.
And for invocable Http-endpoints (which is technically what these are) I prefer Action instead of Controller as it better suites the intention.

Now my proposal is in progress: [Routing][DX] Add full route definition for invokable controller/class WDYT?

maximecolin · 2017-02-22T16:43:59Z

I already use quite the same architecture in my projects (as far as the current Symfony release allow me to go). I will be pleased to see Symfony go this way.

maximecolin · 2017-02-22T19:28:27Z

@mnapoli IMHO views and config are not resources. Views are almost sources and config, well it's config. It won't bother me to have a translations folder at the same level as src/.

ogizanagi · 2017-02-22T19:47:48Z

@maximecolin, @mnapoli : IMHO views and configs are considered resources in a PHP project, as soon as it needs to be located and is consumed by your app.
I actually like the res folder convention, and given the changes done in this PR, I'll find natural to find config, views, translations (,...) in it.

mnapoli · 2017-02-22T19:51:20Z

What's also to consider in this change of putting all resources together (and then I'll stop talking about it) is that it's a first step towards universal resource location (which is actually Puli's goal).

Everything in src/ can be loaded/autoloaded using PSR-4. For the rest, a res/ folder could maybe enable a standard in resource location? Just throwing that idea in there :)

Pierstoval · 2017-02-22T20:02:50Z

And to add my brick in the wall, a res/ folder could be shared between different services and loaded via resource locators in different ways, like Puli proposes 😏

pocky · 2017-02-22T22:32:57Z

@Pierstoval Puli is a great tool but we have to be careful. The project is not very active, it is still in beta, it is much more than a Resource Locator and, unless this has changed, Puli is not yet compatible with Twig 2.x.

So two options, create a Resource Locator à la Puli or contribute to the project.

BTW, I ❤️ Puli and this PR too.

ghost · 2017-02-23T00:02:06Z

is there any way to get rid of conf/autoload.php? doesn't feel like the the right place. Would be nice if composer could handle the annotation registry registration automatically.

pocky · 2017-02-23T07:54:57Z

@jrobeson Good point.

My current directory structure for a single app :

maximecolin · 2017-02-23T08:49:37Z

@ogizanagi Ok, I see what you mean. I'm just quite not confortable with the idea views are not code sources in the same way php files are. But it make more sens with this definition of resources.

linaori · 2017-02-23T13:08:58Z

@dunglas what things like typescript, less, javascript etc, things that need pre-processing? Currently I use app/Resources/assets, but this feels wrong.

kbond · 2017-02-23T13:29:00Z

I have been using the following structure lately and quite like it:

.
├── bin
├── config
├── etc
├── resources
│   ├── assets
│   │   ├── images
│   │   ├── js
│   │   └── sass
│   ├── fixtures
│   ├── migrations
│   ├── translations
│   └── views
├── src
│   ├── AppCache.php
│   ├── AppKernel.php
│   └── autoload.php
├── var
├── vendor
└── web

sstok · 2017-02-23T15:51:38Z

Whats the etc about? Docker and build-stuff?

kbond · 2017-02-23T15:58:25Z

@sstok yeah, anything that doesn't really fit in the other dirs

GromNaN · 2017-02-23T17:13:35Z

src/AppKernel.php

+
+    public function getRootDir()
+    {
+        return __DIR__;


since templates and config are in the parent directory, why not setting the root dir at the root of the repository ?

Because it's kernel's root dir, not project root dir

GromNaN · 2017-02-23T17:18:04Z

templates/default/index.html.twig

@@ -1,4 +1,4 @@
-{% extends 'base.html.twig' %}
+{% extends 'layout.html.twig' %}


I do prefer "base" over "layout". The layout refers to the style.

Also, let's not change anything unnecessarily.

BPScott · 2017-03-06T23:58:13Z

conf/services.yml

+
+        Symfony\Component\Form\FormTypeInterface:
+            tags: ['form.type']
+            public: true  # Mandatory


Can this be removed now that symfony/symfony#21690 is merged and thus form types can be private?

stof · 2017-03-07T09:38:44Z

is there any way to get rid of conf/autoload.php? doesn't feel like the the right place. Would be nice if composer could handle the annotation registry registration automatically.

@jrobeson see symfony/symfony#21837 which is working on it.

Pierstoval · 2017-03-08T12:50:44Z

Just an idea: can't the "action edition" become a new official Symfony edition instead of replacing the Standard Edition?

Symfony CMF already makes updates to his full-stack application from time to time, all based on standard edition, and they seem to agree with it (or maybe it's just me who thinks this 😛 ).

I mean, it has been a bit discussed that the installer could install different Symfony distributions than Demo and Standard Edition (see symfony/symfony-installer#39 ), but actually, the "action pattern" one could easily be one of the different editions.

And it could engage the development process in the installer & the Symfony team on "How to define a Symfony edition", etc.

Obviously, more editions = more maintenance, but as the "action" one will start with Symfony 3.3, this is less maintenance than the SE 😆

stof · 2017-03-08T13:25:40Z

@Pierstoval having multiple official editions has a huge impact on the documentation. So I would not make it an official extension today, especially given that this PR is clearly experimental currently (and relies on unmerged Symfony PRs).
The big maintenance overhead of adding a new edition is not in the installer (where we could easily add support for another one) or in the distribution channel (the building of the SE archive is automated, so this could probably be reused). It is in the need to maintain the documentation twice due to different ways to structure the project.

Pierstoval · 2017-03-08T13:40:40Z

I partially agree, especially about the fact that there is an impact on the documentation.

But actually, a new Symfony edition is maintained by its maintainers, and so should be its documentation, what do you think?

fabpot · 2017-03-08T15:16:00Z

Symfony Distributions are dead. They won't exist in Symfony 4. We won't need them anymore. This will be replaced with Symfony Flex instead. I really have to blog about that as not everybody was at SymfonyCon in Berlin. Will try to do that ASAP to give more background.

Pierstoval · 2017-03-08T15:25:03Z

Symfony Distributions are dead. They won't exist in Symfony 4.

That's great news! 👍

stof · 2017-03-08T15:42:21Z

I really have to blog about that as not everybody was at SymfonyCon in Berlin. Will try to do that ASAP to give more background.

would be great, especially as there is nothing about your keynote available AFAIK: no slides, no video (last time I checked).

fabpot · 2017-04-03T22:54:45Z

Closing this one. Flex is on its way now :)

teohhanhui · 2017-04-04T05:27:29Z

So you're confident that Flex will resolve all of these issues, and that everyone will embrace it? 😄

dunglas force-pushed the ng branch 2 times, most recently from c9b6ef3 to 62cf17c Compare December 16, 2016 12:19

mickaelandrieu reviewed Jan 10, 2017

View reviewed changes

fabpot reviewed Jan 10, 2017

View reviewed changes

dunglas force-pushed the ng branch from 62cf17c to f13b205 Compare January 11, 2017 07:34

dunglas mentioned this pull request Jan 15, 2017

Integrate DunglasActionBundle #960

Closed

dunglas added 2 commits February 21, 2017 20:56

Change the default directory structure and register ActionBundle.

001a728

Update dependencies

Use new 3.3 features

fdce704

dunglas force-pushed the ng branch from f13b205 to fdce704 Compare February 22, 2017 10:26

dunglas changed the title ~~[do not merge] Change the default directory structure and register ActionBundle~~ [do not merge] Change the default directory structure and use new features of 3.3 Feb 22, 2017

dunglas mentioned this pull request Feb 22, 2017

[WIP] make the app/config/services.yml file use the new Symfony autoregistration and autowiring features. symfony/demo#483

Closed

New directory structure

d89d7d2

dunglas force-pushed the ng branch from fa7773e to d89d7d2 Compare February 22, 2017 15:42

yceruto reviewed Feb 22, 2017

View reviewed changes

GromNaN reviewed Feb 23, 2017

View reviewed changes

BPScott reviewed Mar 6, 2017

View reviewed changes

BPScott mentioned this pull request Mar 16, 2017

Remove app/autoload.php #1056

Merged

linaori mentioned this pull request Mar 19, 2017

Documenting a bundle-less setup as of 3.3 symfony/symfony-docs#7655

Closed

fabpot closed this Apr 3, 2017

chalasr mentioned this pull request Aug 31, 2017

Adding standard layout.html.twig for the TwigBundle recipe symfony/recipes#161

Merged

ogizanagi mentioned this pull request Nov 6, 2017

[FrameworkBundle] Add default mapping path for serializer component in bundle-less app symfony/symfony#24833

Merged

ogizanagi mentioned this pull request Nov 18, 2017

[RFC] Revert the move of translations/ into config/ symfony/recipes#253

Merged

ogizanagi mentioned this pull request Jan 10, 2020

move doctrine migrations away from src symfony/recipes#718

Closed

		@@ -1,4 +1,4 @@
		{% extends 'base.html.twig' %}
		{% extends 'layout.html.twig' %}

[do not merge] Change the default directory structure and use new features of 3.3 #1034

[do not merge] Change the default directory structure and use new features of 3.3 #1034

Conversation

dunglas commented Dec 16, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mickaelandrieu Jan 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dunglas commented Jan 11, 2017

lunika commented Jan 13, 2017 • edited Loading

stof commented Jan 13, 2017

chalasr commented Jan 14, 2017 • edited Loading

pocky commented Jan 15, 2017

dunglas commented Jan 15, 2017

dunglas commented Feb 22, 2017 • edited Loading

ruudk commented Feb 22, 2017

dunglas commented Feb 22, 2017

lyrixx commented Feb 22, 2017

mickaelandrieu commented Feb 22, 2017

dunglas commented Feb 22, 2017 • edited Loading

dunglas commented Feb 22, 2017

joelwurtz commented Feb 22, 2017 • edited Loading

yceruto commented Feb 22, 2017 • edited Loading

yceruto Feb 22, 2017 • edited Loading

Choose a reason for hiding this comment

Pierstoval Feb 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yceruto Feb 22, 2017 • edited Loading

Choose a reason for hiding this comment

jvasseur Feb 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yceruto Feb 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sstok Feb 23, 2017 • edited Loading

Choose a reason for hiding this comment

yceruto Feb 23, 2017 • edited Loading

Choose a reason for hiding this comment

maximecolin commented Feb 22, 2017

maximecolin commented Feb 22, 2017 • edited Loading

ogizanagi commented Feb 22, 2017

mnapoli commented Feb 22, 2017

Pierstoval commented Feb 22, 2017

pocky commented Feb 22, 2017

ghost commented Feb 23, 2017

pocky commented Feb 23, 2017

maximecolin commented Feb 23, 2017

linaori commented Feb 23, 2017

kbond commented Feb 23, 2017

sstok commented Feb 23, 2017

kbond commented Feb 23, 2017

Choose a reason for hiding this comment

Pierstoval Feb 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stof commented Mar 7, 2017

Pierstoval commented Mar 8, 2017

stof commented Mar 8, 2017

Pierstoval commented Mar 8, 2017 • edited Loading

fabpot commented Mar 8, 2017

Pierstoval commented Mar 8, 2017

stof commented Mar 8, 2017

fabpot commented Apr 3, 2017

teohhanhui commented Apr 4, 2017

dunglas commented Dec 16, 2016 •

edited

Loading

mickaelandrieu Jan 10, 2017 •

edited

Loading

lunika commented Jan 13, 2017 •

edited

Loading

chalasr commented Jan 14, 2017 •

edited

Loading

dunglas commented Feb 22, 2017 •

edited

Loading

dunglas commented Feb 22, 2017 •

edited

Loading

joelwurtz commented Feb 22, 2017 •

edited

Loading

yceruto commented Feb 22, 2017 •

edited

Loading

yceruto Feb 22, 2017 •

edited

Loading

Pierstoval Feb 22, 2017 •

edited

Loading

yceruto Feb 22, 2017 •

edited

Loading

jvasseur Feb 22, 2017 •

edited

Loading

yceruto Feb 22, 2017 •

edited

Loading

sstok Feb 23, 2017 •

edited

Loading

yceruto Feb 23, 2017 •

edited

Loading

maximecolin commented Feb 22, 2017 •

edited

Loading

Pierstoval Feb 23, 2017 •

edited

Loading

Pierstoval commented Mar 8, 2017 •

edited

Loading