Skip to content
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

WIP: Use ActionView::TemplateDetails for template priority ordering #2145

Closed
wants to merge 1 commit into from

Conversation

sfnelson
Copy link

@sfnelson sfnelson commented Oct 28, 2024

What are you trying to accomplish?

Uses ActionView::TemplateDetails to generate ordering for prioritising templates based on the request/response configuration. See #2128 for details.

What approach did you choose and why?

Using the ActionView::TemplateDetails sort ordering when there are multiple formats. This approach generates a per-request priority array for each template, then sorts the template by the priority order. For example:

Template details:
* test_component.html.erb => { locale: nil, format: :html, variant: nil, handler: :erb }
* test_component.html+admin.erb { local: nil, format: :html, variant: :admin, handler: :erb }
* test_component.html+phone.erb => { locale: nil, format: :html, variant: :phone, handler: :erb }
* test_component.json.jbuilder => { locale: nil, format: :json, variant: nil, handler: :builder }
* test_component.json+admin.jbuilder => { locale: nil, format: :json, variant: :admin, handler: :builder }

Request details (generated by Rails from lookup_context for a turbo stream request with variants [:phone, :admin]):
formats: {turbo_stream: 0, html: 1, nil: 2}
locale: {en: 0, nil: 1}
variants: {phone: 0, admin: 1, nil: 2}
handlers: {raw: 0, erb: 1, html: 2, builder: 3, ruby: 4, nil: 5}

Priorities: (format, locale, variant, handler)
* test_component.html.erb => [1, 1, 2, 1]
* test_component.html+admin.erb => [1, 1, 1, 1]
* test_component.html+phone.erb => [1, 1, 0, 1]
* test_component.json.jbuilder # skipped because format is not acceptable
* test_component.json+admin.jbuilder # skipped because format is not acceptable

In this situation, the format parameter is restricted to turbo_stream, html, or unset (nil), so the jbuilder templates are excluded. The locales are the same (unspecified) so the second term is not relevant. Finally, the phone variant is chosen because the phone variant parameter (0) is lower than the admin variant (1) or the unspecified variant (2). In this situation the handler priority doesn't matter.

Anything you want to highlight for special attention from reviewers?

Two failing tests:

  • changes the number of allocations per render,
  • not compatible with the request-format approach to formats added in 3.15

TODO:

  • clean up __vc_variant – this approach is limited as it only supports one variant instead of the intended cascade
  • work out what to do with the (now unsed) variant and format arguments to render_template_for
  • review performance and allocations impact of this approach
  • remove or refactor the __vc_request which bakes the request format in to the response, instead of using the allow header

Uses ActionView::TemplateDetails to generate ordering for prioritising
templates based on the request/response configuration.

Note that this approach changes the number of allocations per render,
and is not compatible with the request-format approach implemented in
3.15.

TODO:
* clean up `__vc_variant` in @template – this approach is limited as it only supports one variant instead of the intended cascade
* work out what to do with the (now unsed) variant and format arguments to render_template_for
* review performance and allocations impact of this approach
* remove or refactor the `__vc_request` which bakes the request format in to the response, instead of using the allow header
@sfnelson sfnelson closed this Nov 6, 2024
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.

1 participant