Skip to content

Conversation

@alexandre-daubois
Copy link
Member

@alexandre-daubois alexandre-daubois commented Dec 19, 2025

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

This is extracted from #61517 to make things more atomic. This PR introduces the functions provider and converts built-in "hard-coded" JSON path functions to their proper classes. This allows better responsibility separation, better testing as well as functions lazy loading.

The integration with FrameworkBundle and a new AsJsonPathFunction attribute will be done in a followup PR.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Dec 22, 2025

I'm not super convinced we need this. The RFC defines the functions so hardocding the behavior is required. Testing internal API (the new classes) doesn't really improve overall coverage either to me. We already (or should) ensure we test the behavior of the user-land exposed API of course.
Lazy loading also doesn't make a strong argument to me. We're treading a few inlined opcodes with the sames wrapped in more class-related boilerplate.
That my sentiment here :)

@alexandre-daubois
Copy link
Member Author

The idea is to improve maintainability of built-in functions by extracting them from an already pretty big file such as JsonCrawler. The idea is also to have one dedicated mechanism to load and execute any function (custom or built-in one) instead of dealing with two different loading/executing logic, depending on the fact its builtin or not.

Indeed, it doesn't change anything for people using the crawler, but on the maintainer side, we will only have one logic to deal with. Responsibilities are better separated. I understand that lazy-loading is not the argument here, but I think that's a nice-to-have worth mentioning

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