More updates for "Type-based" injection #7906

weaverryan · 2017-05-14T19:22:37Z

Apologies for another big PR, and also if I'm stepping on anyone else's feed - I'm just trying to charge through and make all the changes to the docs. This contains 2 things:

More updates to entries to favor type-based injection (type-hinting and relying on autowiring) over manually configuration. I have more chapters to do still, but I'm mostly done.
Important updates (mostly to service_container.rst) now that we're importing all of src/AppBundle in services.yml (Making *all* classes in src/AppBundle available as services symfony-standard#1070). Side note: wooohooo!!!

P.S. I deleted one entry that I thought was really poor and unnecessary.

…g all of src/AppBundle

javiereguiluz

Ryan, thanks for this nice PR. The way you added small tips and help notes for those not using autoconfigure is great and I think people won't have any trouble finding the new docs. Thanks!

javiereguiluz · 2017-05-15T06:55:26Z

event_dispatcher/before_after_filters.rst

-implements ``TokenAuthenticatedController``, token authentication is
-applied. This lets you have a "before" filter on any controller that you
-want.
+    If your subscriber id *not* called on each request, double-check that


id -> is

javiereguiluz · 2017-05-15T07:10:27Z

form/create_custom_field_type.rst

-First, add a ``__construct`` method to ``GenderType``, which receives the gender
-configuration::
+If you need to access :doc:`services </service_container>` from your form class,
+just a ``__construct()`` method like normal::


just -> use

javiereguiluz · 2017-05-15T07:14:44Z

form/create_form_type_extension.rst

-The ``extended_type`` key of the tag is the type of field that this extension should
-be applied to. In your case, as you want to extend the ``Symfony\Component\Form\Extension\Core\Type\FileType``
-field type, you will use that as the ``extended_type``.
+The ``extended_type`` key of the tag must match the class you're returning from


This looks wrong. Why repeat in extended_type exactly the same information I configured in getExtendedType() method? One of those two should be gone.

I completely agree. But, that's the way the feature is built currently - I just double-checked it. I think that getExtendedType() should be static and this extended_type should not be needed (exactly how an event subscriber works).

javiereguiluz · 2017-05-15T07:37:20Z

form/form_dependencies.rst

@@ -71,36 +71,41 @@ Alternatively, you can define your form class as a service. This is a good idea
 you want to re-use the form in several places - registering it as a service makes
 this easier.

-Suppose you need to access the ``doctrine.orm.entity_manager`` service so that you
+Suppose you need to access the ``EntityManager`` service so that you


All this is still confusing to me but ... is this statement correct? --> [...] the EntityManager service [...]

If I execute ./bin/console debug:container --types I see this:

-------------------------------------------- ------------------------------------------------ Service ID Class name -------------------------------------------- ------------------------------------------------ Doctrine\Common\Annotations\Reader alias for "annotations.cached_reader" Doctrine\Common\Persistence\ManagerRegistry alias for "doctrine" Doctrine\Common\Persistence\ObjectManager alias for "doctrine.orm.default_entity_manager" Doctrine\DBAL\Driver\Connection alias for "doctrine.dbal.default_connection"

So there's no "EntityManager service", right?

There is also an EntityManagerInterface result:

Doctrine\ORM\EntityManagerInterface alias for "doctrine.orm.default_entity_manager"

But you're right. Saying:

I need to access the EntityManagerInterface service

sounds weirder. Technically the concrete instance is EntityManager, but it's true that you don't see that unless you specifically look at the details for doctrine.orm.default_entity_manager.

So there's no "EntityManager service", but there is an EntityManager object in the container... which is what the user wants (and this is a service).

Anyways, I've made a change, let me know what you think - we should continue tweaking. I think it's important because we need to be able to find clear language to explain things, otherwise there's a problem :)

javiereguiluz · 2017-05-15T07:37:58Z

form/form_dependencies.rst

-Next, register this as a service and tag it with ``form.type``:
+If you're using :ref:`autowire <services-autowire>` and
+:ref:`autoconfigure <services-autoconfigure>`, then you don't need to do *anything*
+else: Symfony will automatically know to pass the entity manager service to your


Here again: "the entity manager service"

javiereguiluz · 2017-05-15T07:41:24Z

service_container/parameters.rst

+            // But % does need to be escaped
+            $container->setParameter('url_pattern', 'http://symfony.com/?foo=%%s&amp;bar=%%d');
+
+    .. code-block:: yaml


This empty code-block should be removed.

weaverryan · 2017-05-16T10:30:41Z

Thanks for the review Javier and the nice words about the notes! I was hoping to have a sanity check from someone else on the clarity :).

GuilhemN

That's really great! I missed it so didn't review it before :)

GuilhemN · 2017-05-16T15:18:36Z

profiler/data_collector.rst

-``data_collector``:
+If you're using the :ref:`default services.yml configuration <service-container-services-load-example>`
+with ``autoconfigure``, then Symfony will automatically see your new data collector!
+Your ``collect()`` method should be called next time your refresh.


GuilhemN · 2017-05-16T15:22:02Z

service_container.rst

-                http://symfony.com/schema/dic/services/services-1.0.xsd">
+    The documentation assumes you're using
+    `Symfony Standard Edition (version 3.3) services.yml`_ configuration. The most
+    important part is this:


As we're using actions with services as arguments almost everywhere, isn't the controller part important too?

GuilhemN · 2017-05-16T15:34:27Z

form/data_transformers.rst

-
-No problem! Just add a ``__construct()`` function to ``TaskType`` and force this
-to be passed in by registering ``TaskType`` as a service::
+Next, you need to use the ``IssueToNumberTransformer`` object inside if ``TaskType``


GuilhemN · 2017-05-16T15:36:11Z

form/form_collections.rst


        // ...
-        public function editAction($id, Request $request)
+        public function editAction($id, Request $request, EntityManagerInterface $e,)


too many commas :)

GuilhemN · 2017-05-16T15:38:54Z

logging/formatter.rst

        $container->loadFromExtension('monolog', array(
            'handlers' => array(
                'file' => array(
                    'type'      => 'stream',
                    'level'     => 'debug',
-                    'formatter' => 'my_formatter',
+                    'formatter' => JsonFormatter::class',


use statement missing (it is documented in config.php, a few lines above).

HeahDude · 2017-07-25T15:59:32Z

event_dispatcher/method_behavior.rst

-            $event = new FilterBeforeSendEvent($foo, $bar);
-            $this->dispatcher->dispatch('foo.pre_send', $event);
+            // dispatch an event before the method
+            $event = new BeforeSendMailEvent($subject, $message);


BeforeSendingMailEvent would be better, but regarding the "real" event name what about PreSendMailEvent?

HeahDude · 2017-07-25T16:00:13Z

event_dispatcher/method_behavior.rst


            // the real method implementation is here
            $ret = ...;

            // do something after the method
-            $event = new FilterSendReturnValue($ret);
-            $this->dispatcher->dispatch('foo.post_send', $event);
+            $event = new AfterSendMailEvent($ret);


Same here, AfterSendingMailEvent would be better.

HeahDude · 2017-07-25T16:06:29Z

form/dynamic_form_modification.rst

@@ -385,7 +352,7 @@ class, you can simply call::
        }
    }

-You can also easily embed the form type into another form::
+You can also  embed the form type into another form::


Extra space.

HeahDude · 2017-07-25T16:08:32Z

service_container.rst

@@ -15,7 +15,7 @@ a very special object called the **service container**. If you have the service
 then you can fetch a service by using that service's id::

    $logger = $container->get('logger');
-    $entityManager = $container->get('doctrine.entity_manager');
+    $entityManager = $container->get('doctrine.orm.entity_manager');


Should be backported in 2.7.

Fixed in #8228.

HeahDude · 2017-07-25T16:33:15Z

service_container.rst

+    automatically removed from the final container. In reality, the import simply
+    means that all classes are "available to be *used* as services" without needing
+    to be manually configured.
+


Maybe we could also note that value object classes with type hinted constructor should be explicitly excluded to avoid issues at compile time.

weaverryan added 3 commits May 14, 2017 15:19

more tweaks

0a6f24d

More updates

5c9ac78

Many more updates to favor types of ids' AND updates for now importin…

ba77a15

…g all of src/AppBundle

carsonbot added the Status: Needs Review label May 14, 2017

More changes

f7f9178

javiereguiluz approved these changes May 15, 2017

View reviewed changes

javiereguiluz added Status: Reviewed and removed Status: Needs Review labels May 15, 2017

weaverryan added 2 commits May 16, 2017 06:28

Tweaks

1b82e39

fixing bad table

b82f4af

weaverryan added 4 commits May 16, 2017 06:33

fixing bad link

11b9627

more bad links

220f419

bad link

1de941c

Bad link

228b0de

weaverryan mentioned this pull request May 16, 2017

Update best_practices/templates.rst #7902

Merged

weaverryan closed this in e437d9d May 16, 2017

weaverryan deleted the more-types-round2 branch May 16, 2017 10:49

GuilhemN reviewed May 16, 2017

View reviewed changes

HeahDude reviewed Jul 25, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More updates for "Type-based" injection #7906

More updates for "Type-based" injection #7906

weaverryan commented May 14, 2017

javiereguiluz left a comment

javiereguiluz May 15, 2017

javiereguiluz May 15, 2017

javiereguiluz May 15, 2017

weaverryan May 16, 2017

javiereguiluz May 15, 2017

weaverryan May 16, 2017

javiereguiluz May 15, 2017

javiereguiluz May 15, 2017

weaverryan commented May 16, 2017

GuilhemN left a comment

GuilhemN May 16, 2017

GuilhemN May 16, 2017

GuilhemN May 16, 2017

GuilhemN May 16, 2017

GuilhemN May 16, 2017 •

edited

Loading

HeahDude Jul 25, 2017

HeahDude Jul 25, 2017

HeahDude Jul 25, 2017

HeahDude Jul 25, 2017

HeahDude Jul 26, 2017

HeahDude Jul 25, 2017

More updates for "Type-based" injection #7906

More updates for "Type-based" injection #7906

Conversation

weaverryan commented May 14, 2017

javiereguiluz left a 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

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

weaverryan commented May 16, 2017

GuilhemN left a 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

Choose a reason for hiding this comment

GuilhemN May 16, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuilhemN May 16, 2017 •

edited

Loading