Skip to content

Conversation

noelforte
Copy link
Contributor

@noelforte noelforte commented Apr 28, 2025

This change motivated by: noelforte/eleventy-plugin-vento#221

I couldn't figure out why the i18n plugin's presence was breaking the getNextCollectionItem and getPreviousCollectionItem filters since the Vento template engine plugin doesn't do anything fancy with the filters.

After a lot of trial and error, I discovered that this search for collections.all at the very end of the GetLocaleCollectionItem module checks a lot of things specific to Eleventy's own internal engines but of course custom engines don't add a ctx or context object, or even this.collections, since this.page and this.eleventy are the only context keys Eleventy supports, and collections is already accessible in page data anyway.

Rather than add this.collections to the Vento custom engine, could we simplify the all declaration to search the data key instead since the data key is already available on the render context anyway?

If not, I can look into adding this.collections to the custom engine for my plugin, but if this can be handled upstream that puts less pressure on custom engines to support adding it.

Thanks Zach!

@noelforte
Copy link
Contributor Author

noelforte commented May 30, 2025

Sorry to bug you Zach, any inclination on when this might get reviewed? Is there something that I could do to help push things along?

@noelforte noelforte force-pushed the i18n-page-resolution branch from faa4a92 to 0c16992 Compare July 7, 2025 18:46
@noelforte
Copy link
Contributor Author

@zachleat Just confirming, is this PR on hold for a specific reason? It's not been tagged as on-hold, and also I haven't heard anything about getting this reviewed apart from one reference in #2844. I know you're busy but I'd like to know whether I should be implementing a workaround on my own or whether the 11ty codebase can adapt these changes.

this.ctx?.collections?.all ||
this.context?.environments?.collections?.all ||
[];
let all = this.collections?.all || this.data?.collections?.all || [];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll need to confirm on my end, but I can't remember whether this.data is something provided by Eleventy, or something that is being injected by a custom engine in my use case. If this.data is Eleventy-supplied, then that's all that needs including in order to make custom engines pick up the locale page object.

The removal of lines 40-42 I did was mostly to reduce redundancy while (hopefully) not breaking backwards compatibility, but do let me know if I'm wrong!

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