Skip to content

This library collides with the php extension jsonpath #70

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

Open
dcmals opened this issue Feb 14, 2025 · 3 comments
Open

This library collides with the php extension jsonpath #70

dcmals opened this issue Feb 14, 2025 · 3 comments

Comments

@dcmals
Copy link

dcmals commented Feb 14, 2025

Reported with PHP version: PHP 8.3.17

If the PHP module jsonpath is enabled, when trying to instantiate the JsonPath\JsonPath class, it ends up loading the one from the PHP module, not the Galbar one.

Before disable jsonpath module

Image

After disable jsonpath module

Image

@TheDigitalOrchard
Copy link
Contributor

TheDigitalOrchard commented Mar 26, 2025

Namespace issue. This library is not following the standard Vendor\Package naming convention, so that could be fixed in a future release. Maybe create a PR with the suggested changes?

The JSONPath module/extension doesn't seem to be very widely used? I hadn't heard of it before reading this issue.

  • Not included with MacPorts PHP libraries
  • Not included with popular Ubuntu APT libraries.

I'm sure it's easy to obtain and install, but with it not being available by default with the above package managers suggests that it's not widely installed? Do you have a link to the source for that module?

@TheDigitalOrchard
Copy link
Contributor

I created a PR #73 that would address this issue. Would require a major version bump.

@Galbar
Copy link
Owner

Galbar commented Mar 27, 2025

I'm sorry. I had missed this issue. Thanks @TheDigitalOrchard for the PR. This is an important change, as you suggest it requires a major version bump. Given how unpopular the jsonpath extension is, I'd rather wait for a bigger functionality change before publishing such a big change.

Maybe after getting #71 right, we could get the tests working in 7.x and then merge this and publish a new major.

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

No branches or pull requests

3 participants