Error when using page variables (`TypeError: Jekyll::Drops::DocumentDrop does not have #dig method`)

[fabacab]

First, sorry for having just said everything was kosher and then finding this bug immediately after. Turns out the minute I tried to use a page variable (which my templates were not doing yet; they were still doing {% assign url = page.some_var %}{% ical url: url %}…), I get the error quoted in the title.

Template code like this:

{% ical url: page.some_var %}{% endical %}

fails to build with the following exception:

$ bundle exec jekyll server --future
[…]
  Liquid Exception: Jekyll::Drops::DocumentDrop does not have #dig method in /_layouts/calendars.html
jekyll 3.8.5 | Error:  Jekyll::Drops::DocumentDrop does not have #dig method

A bundle exec jekyll build --trace points precisely to the error, described here.

The issue arises from lack of support for Hash#dig in Jekyll::Drops::DocumentDrop, which is what a call to, e.g., context.registers.dig(:page) is actually returning. It doesn't return a plain Ruby Hash. This means any second argument specified at jekyll-ical-tag.rb line 123 will cause an error when the register is the :page or :site register, but works as expected in the one case of the most local ({% ical %}) scope.

I can find some time to code a solution at some point later this week but figured I'd report the issue immediately.

[rickychilcott]

It looks like fetch is supported in DocumentDrop see https://github.com/jekyll/jekyll/pull/5056

I'm not sure I have a full test case, can you send me a minimal viable test case to make sure I add it to my project I'm using for testing? #18 might fix the issue, but I'm not 100% sure.

[fabacab]

Thanks again. #18 is an improvement and seems to resolve the issue I'm having in at least some cases. Other feeds still raise issues, but they may be unrelated? I've put together a test case (issue-17.testcase.tar.gz), though, that shows another issue, and I'm including it here in case it's useful for you to make this next patch more robust.

So, attached to this comment is a tarball with a minimal viable test case. Due to Bundler's prioritzation of Gem sources, you have to do the following to run the test case:

mkdir -p /tmp/issue-17-testcase
tar -xvzf ~/Downloads/issue-17.testcase.tar.gz -C !$ # Or wherever.
cd !$
bundle install
# Then, after Jekyll is installed, uncomment the `:git` and `:branch` lines in the Gemfile.
sed -e 's/^#//' -e '3s/$/,/' -i '' Gemfile
bundle install # A second time, this time using the code from the hotfix attemp.
bundle clean # Hygiene, just in case.
bundle exec jekyll build --trace

This will attempt to use an example calendar from Google Calendar as well as one from Meetup.com.

[rickychilcott]

Thanks for this example. I'm really surprised that we have to handle all of these individual cases. I feel like Jekyll should do all of the dereferencing for us.

I'm going to do some investigation as to how other plugins do it. It seems as though I may have made a miss-step by making the API {% ical url: https://space.floern.com/launch.ics %} instead of {% ical https://space.floern.com/launch.ics %}

Most other Jekyll tags use the later form.

[rickychilcott]

Holy cats, we were playing on hard mode.

After reviewing https://github.com/jekyll/jekyll-gist/blob/master/lib/jekyll-gist/gist_tag.rb#L20-L43 I think I have a fix. See #18. Mind testing for all your cases?

[fabacab]

Holy cats, we were playing on hard mode.

After reviewing https://github.com/jekyll/jekyll-gist/blob/master/lib/jekyll-gist/gist_tag.rb#L20-L43 I think I have a fix. See #18. Mind testing for all your cases?

Wowzers, yeah, that is a LOT easier. I just merged your commit 471999ce6d0ad70d4235fc90629a5336face323e to my dev env and it works without issue. My templates are still using some Liquid-level workarounds, but for the time being this doesn't seem to introduce any unexpected issues, and the code is way simpler, so I say go for it.

As I adjust my templates in the future to be more intuitive from a front-end integration perspective, I'll report any new issues if I get hit by them.

[rickychilcott]

Version 1.2.0 should have all of the fixes