Reset default EntrypointLookup on exception to fix #21 and #73 #74
Conversation
Wouldn't it be possible to handle that using the
|
|
I made it work like you suggested. I still don't love it, but I like it a lot better than before. |
| { | ||
| private $entrypointLookupCollection; | ||
|
|
||
| private $entrypointNames; |
There was a problem hiding this comment.
I believe these are buildNames - we should change entrypoint -> build everywhere in this class
| @@ -0,0 +1,26 @@ | |||
| <?php | |||
|
|
|||
| namespace Symfony\WebpackEncoreBundle\EventListener; | |||
There was a problem hiding this comment.
can you add LICENSE header at the top?
| $this->entrypointLookupCollection->getEntrypointLookup($entrypointName)->reset(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Could we add a very basic unit test for this class? There isn't much to test, but having at least ONE basic test will protect us from making any super silly typos now or in the future.
src/Resources/config/services.xml
Outdated
| <service id="webpack_encore.exception_listener" class="Symfony\WebpackEncoreBundle\EventListener\ExceptionListener"> | ||
| <tag name="kernel.event_listener" event="kernel.exception" /> | ||
| <argument type="service" id="webpack_encore.entrypoint_lookup_collection" /> | ||
| <argument /> <!-- build list of entrypoint paths --> |
There was a problem hiding this comment.
| <argument /> <!-- build list of entrypoint paths --> | |
| <argument /> <!--collection of build names --> |
|
@weaverryan should be all done, can you take a look? |
|
@weaverryan @Lyrkan this has been waiting for review/merge almost three months now, can we please move it along? |
|
Sorry for the long delay @tbmatuka - but thank you SO much for finishing this :). Cheers! |
Since @ckrack seems to be unavailable to continue working on #21, I figured this would be faster :)
I don't like having
_defaulthardcoded in the listener, but I see no other options right now.I thought about adding a
resetAll()method to EntrypointLookupCollection, but there were a couple of issues with that:has()andget()methods, so I couldn't go through it. This would also have to be replaced (and break BC) to implementresetAll().Fixes symfony/demo#910