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 jekyll 4.0 cache mismatch errors #182

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mohkale
Copy link
Contributor

@mohkale mohkale commented Mar 17, 2020

Jekyll 4.0 introduced a new caching mechanism to the LiquidRenderer class. This remembers the paths of any files passed to convert and the rendered output.

The issue here is that Jekyll-Paginate-V2 sets the name of all Jekyll::PaginateV2::Generator::PaginationPage instances to index.html. Meaning every pagination page gets rendered to the result of the first paginated page.

This patch simply assigns some fields the Page instance, such that it appears to have the same file name as the file to which it's written to (in the _site directory).

Jekyll 4.0 introduced a new caching mechanism to the LiquidRenderer
class. This remembers the paths of any files passed to convert and
the rendered output. The cache is reset between repeat invocations
of `Site.process' it's really only to prevent needing to re render
templates or includes.

The cache works by remembering the file name of any files being
rendered, returning the cached value on demand should jekyll
try to render a file with the same path (from the src directory)
multiple times.

The issue here is that Jekyll-Paginate-V2 sets the name of all
`Jekyll::PaginateV2::Generator::PaginationPage` instances to
index.html. Meaning every pagination page gets rendered to the result
of the first paginated page.

This patch simply assigns some fields the Page instance, such that it
appears to have the same file name as the file to which it's written
to (which should guarantee it's uniqueness and prevent liquid cache
mismatches).
@mohkale mohkale changed the title paginationPage.rb: fix jekyll4 cache mismatch errors fix jekyll 4.0 cache mismatch errors Mar 17, 2020
@@ -41,6 +42,8 @@ def initialize(page_to_copy, cur_page_nr, total_pages, index_pageandext)
end

def set_url(url_value)
@path = url_value.delete_prefix '/'
Copy link
Contributor

Choose a reason for hiding this comment

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

Caution: String#delete_prefix was added only in Ruby 2.5. Therefore, this will break on older Ruby versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this seem better to you?

if path[0] == '/'
  path[1..]
end

@ashmaroli
Copy link
Contributor

@mohkale Can you post the before and after screenshots of a verbose build (build with --verbose switch) for a test-site to be certain of this patch..

@mohkale
Copy link
Contributor Author

mohkale commented Mar 18, 2020

Kay, will get on it soon.

@mohkale
Copy link
Contributor Author

mohkale commented Mar 18, 2020

@ashmaroli I've attached to this the before and after log and also a zip of the site.

The site has two collections (foo & bar) each of which has 2 index pages contents/foo.markdown, contents/bar.markdown. If you build as it is, in the _site directory foo/index.html will have the exact same contents as bar/index.html (because bar was rendered first and the liquid cache doesn't see the two as distinct from each other).

@ashmaroli
Copy link
Contributor

@mohkale The before and after diff shows:

-          Rendering: .html
-   Pre-Render Hooks: .html
-   Rendering Liquid: .html
-   Rendering Markup: .html
-   Rendering Layout: .html
+          Rendering: bar/.html
+   Pre-Render Hooks: bar/.html
+   Rendering Liquid: bar/.html
+   Rendering Markup: bar/.html
+   Rendering Layout: bar/.html
       Layout source: site
-          Rendering: .html
-   Pre-Render Hooks: .html
-   Rendering Liquid: .html
-   Rendering Markup: .html
-   Rendering Layout: .html
+          Rendering: foo/.html
+   Pre-Render Hooks: foo/.html
+   Rendering Liquid: foo/.html
+   Rendering Markup: foo/.html
+   Rendering Layout: foo/.html

foo/.html and bar/.html still don't look right to me..

@mohkale
Copy link
Contributor Author

mohkale commented Mar 18, 2020

@ashmaroli could you clarify what you mean? do you mean the filename (index) should also be included.

@ashmaroli
Copy link
Contributor

Yes. Or some other unique name.

@mohkale
Copy link
Contributor Author

mohkale commented Mar 19, 2020

I'm planning to switch from jekyll to hugo, so I'm not sure when (or if) I'll get back round to trying to fix this. If someone wants to try to do so, feel free 😄.

@jameshi16
Copy link

hi, I'll give it a shot 😄

Thanks for your work!

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.

3 participants