Bug: `jsonapi` and `trim` extras break Pagy nav helpers when used simultaneously

[marcus-deans]

👀 Before submitting...

• [x] I upgraded to pagy version 9.3.4
• [x] I searched through the Documentation
• [x] I searched through the Issues
• [x] I searched through the Q&A
• [x] I asked Pagy Trained AI - Warning - The information may not be 100% accurate, but it serves as a good starting point for further investigation.

🧐 REQUIREMENTS

• [x] I am providing a VALID code file that confirms the bug
• [x] I am NOT posting any of the USELESS THINGS listed above
• [x] I am aware that this issue will be automatically closed if the code file is missing or INVALID

💬 Description

Using default configuration for jsonapi and trim at the same time breaks Pagy nav helpers:

I have attached an edited demo.ru file, but for convenience, the only changes I made was adding

+ require 'pagy/extras/jsonapi'
+ # Pagy::DEFAULT[:trim_extra] = false # opt-in trim
- Pagy::DEFAULT[:trim_extra] = false # opt-in trim

Thus, the default configuration for both jsonapi and trim is implemented.

The demo app works fine for the initial load. It then breaks for any subsequent pages when trying to load the prior page's URL. This is observable below

https://github.com/user-attachments/assets/c2d943ea-fdd5-4b7c-9b35-49fa5e2a680a

[marcus-deans]

I understand the report includes these guidelines:

🚩 Avoid posting USELESS THINGS like:

• Code snippets / Log transcripts
• Descriptions of errors / behaviors / expectations
• Rationalizations about WHY you THINK your problem is a pagy bug

So did not include this in my actual report. However, I believe I can directly identify the problem and would like to save your debugging time, if helpful.

The issue is that jsonapi automatically overrides the pagy_set_query_params method, such that page params throughout the application become nested

module UrlHelperOverride
      # Override UrlHelper method
      def pagy_set_query_params(page, vars, query_params)
        return super unless vars[:jsonapi]

        query_params['page'] ||= {}
        query_params['page'][vars[:page_param].to_s]  = page
        query_params['page'][vars[:limit_param].to_s] = vars[:limit] if vars[:limit_extra]
      end
    end
    UrlHelpers.prepend UrlHelperOverride

In trim, we have regex expecting the page_param variable, which is still :page as default. However, it is now nested, and the regex fails. This thus returns nil, and in the case of the demo app, crashes. In my own setup, the "Previous" button in the helpers becomes "null".

 def pagy_trim(pagy, a)
      a.sub!(/[?&]#{pagy.vars[:page_param]}=1\b(?!&)|\b#{pagy.vars[:page_param]}=1&/, '')
    end

I am happy to PR at minimum documentation of this interaction in the "Interaction with other features/extras" section of the jsonapi extra docs, so that others can identify this behaviour separately.

I noted that jsonapi reserves 'page' as a top level param for itself. I thus think inspecitng pagy.vars[:jsonapi] in #pagy_trim would allow the regex to be implemented appropriately based on the scenario. I'm not fully aware of the context of variable propagation, particularly when using non-global configuration toggles for these, so perhaps that is not viable. I am happy to attempt that small change, if that is the agreed upon route.

[ddnexus]

I am terribly late on all the PRs... sorry, but I have been abroad solving some family problem, and focusing on the development of the new major version. You can see the rc1 here.

About the trim extra... here is the current status: https://github.com/ddnexus/pagy/discussions/761#discussioncomment-12349254

A PR could probably be used for v9+, but I would have no time for actually following its merging... I close this bug as "wontfix" ATM.

Thanks for your report.