Skip to content

Fix webpack_encore.entrypoint_lookup registration #39

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

Closed
wants to merge 1 commit into from
Closed

Fix webpack_encore.entrypoint_lookup registration #39

wants to merge 1 commit into from

Conversation

MrMitch
Copy link

@MrMitch MrMitch commented Feb 1, 2019

The webpack_encore.entrypoint_lookup service instanciation was throwing an exception, preventing it from being used properly with dependency injection.

Closes #36

@MrMitch
Copy link
Author

MrMitch commented Feb 4, 2019

The tests pass, the failure is due to a deprecation warning that was not introduced by this PR :

The "PHPUnit_Framework_MockObject_Matcher_ConsecutiveParameters::verify()" method will
require a new "PHPUnit_Framework_MockObject_Invocation $invocation" argument in the next
major version of its parent class "PHPUnit_Framework_MockObject_Matcher_StatelessInvocation", 
not defining it is deprecated.

The webpack_encore.entrypoint_lookup service
instanciation was throwing an exception,
preventing it from being used properly with
dependency injection.
@weaverryan
Copy link
Member

Hey @MrMitch!

I noticed this a few days ago too - that service is totally broken. The reason we didn't notice is that... if you look closely, it's not used inside the bundle anymore! It's even passed to webpack_encore.twig_entry_files_extension, but that service doesn't actually use it.

The proper solution is to not use this service at all anymore - and to use webpack_encore.entrypoint_lookup_collection instead and then fetch out the default entrypoint lookup.

Would you mind re-working this PR to actually completely remove the webpack_encore.entrypoint_lookup service?

@weaverryan
Copy link
Member

Thanks for opening this @MrMitch. As I mentioned earlier, we've actually removed the broken service entirely. This was a really nice PR though - I like the integration tests you added. I hope we'll see you again here :)

@weaverryan weaverryan closed this Mar 1, 2019
@MrMitch
Copy link
Author

MrMitch commented Mar 4, 2019

Hi @weaverryan, thank you for the feedback. I've had no time to circle back to this and the solution you implemented in #45 seems more elegant and more flexible.
Any reason why it couldn't get included in the recent 1.2.0 or 1.2.1 releases ?

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

Successfully merging this pull request may close these issues.

2 participants