Skip to content

Conversation

ollie-nye
Copy link
Contributor

Closes rails/rails#44403

If a route is defined that generates the helper asset_path, the view_context passed to importmap.to_json will use the resource route instead of the asset pipeline route. By default, this is aliased to path_to_asset, so updating this method to use that instead resolves the issue.

@ollie-nye ollie-nye force-pushed the prevent-asset-path-clashing-with-routes branch from d6c3bc1 to 5f544b5 Compare February 11, 2022 18:17
Copy link
Member

@gmcgibbon gmcgibbon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the contract of a resolver and is likely a breaking change, but I think it makes sense 👍

@matthewd
Copy link
Member

Yeah, this is technically a breaking change of the expected API, but in practice to me it seems early enough in the library's lifetime that we can reasonably assume currently-extant callers are almost certainly passing a Rails-sourced object that already meets this criteria.

We could retain compatibility with a respond_to? check and a fallback, but it really just seems like avoidable complication.

@matthewd matthewd merged commit b3856c5 into rails:main Feb 12, 2022
@ollie-nye ollie-nye deleted the prevent-asset-path-clashing-with-routes branch February 12, 2022 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants