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

Fix url filters, caching key was independent on baseurl #8275

Closed
wants to merge 1 commit into from
Closed

Fix url filters, caching key was independent on baseurl #8275

wants to merge 1 commit into from

Conversation

torrocus
Copy link
Contributor

@torrocus torrocus commented Jul 1, 2020

This is a 🐛 bug fix.

Summary

PR #7793 was optimizing URL filters using cache. But there didn't take into account the possibilities of different baseurl in one project. This case occurs when many languages ​​are used.
Now cache key is depend on baseurl and input path.
In addition, I added tests for these cases.

Context

It's not related to any issue in this repository.
But working with Jekyll plugins it's related to kurtsson/jekyll-multiple-languages-plugin#168

@ashmaroli
Copy link
Member

@torrocus I understand that a change introduced in v4.1.0 broke a plugin but I don't understand this concept of multiple baseurl values for the same project. Please elaborate..

@ashmaroli
Copy link
Member

The proposed implementation here would result in unnecessary allocation of arrays and strings for all users irrespective of whether they use the plugin.
So as of now, my stand is that since the plugin mutates the value of site.config["baseurl"], the plugin should implement its own override for URL Filter cache...

/cc @jekyll/core

@torrocus
Copy link
Contributor Author

torrocus commented Jul 1, 2020

@ashellwig Perhaps I did not quite express myself correctly. The point is that multi-language plugins build pages for each language separately. And then the baseurl in the configuration changes. That's why when I debugged, I saw baseurl change.

Ok, I will understand that the patch should be in the plugin.

@torrocus torrocus closed this Jul 2, 2020
@jekyll jekyll locked and limited conversation to collaborators Jul 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants