Skip to content

[JsonPath] Add CrawlerFactoryInterface and JsonCrawlerFactory #60624

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

Conversation

alexandre-daubois
Copy link
Member

Q A
Branch? 7.4
Bug fix? no
New feature? yes
Deprecations? no
Issues -
License MIT

New try after #60083.

To propose the best integration with FrameworkBundle and upcoming features (like injecting new custom functions for filtering), we need a way to inject a crawler in services.

This PR proposes a new factory like the other PR, without the FrameworkBundle integration. We'll do this in a second time.

There were many discussions on which approach to choose: either a separate factory or a factory directly included in the Crawler itself. It seems the separate factory is the preferred way, but no consensus is found yet. I personally prefer the separate factory as well, but the debate is still open 🙂

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jun 5, 2025

I'm really doubtful about this approach. We discussed in #60083 that crawlers can vary by input format, so that one factory service or interface cannot rule them all.
The need for a factory is to support functions you wrote. If that's The use case for having a service, then we'd need to have support for functions. Without a use case in core, there's no need for any design pattern overhead.
Then, assuming function is really the use case, what about an approach where the functions themselves are provided as a service (or a collection/container of services)? Then one would inject this "functions provider" and then instantiate the exact implementation they need: eg new JsonCrawler($data, $functionsProvider)

@alexandre-daubois
Copy link
Member Author

I really like the approach of functions provider being the service instead of the crawler or a factory. I'm having a look at it!

@alexandre-daubois
Copy link
Member Author

Let's close this one. I'll definitely implement the functions provider mechanism, but I won't work on this one before the crawler is compliant with the RFC test suite. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants