Skip to content

Commit

Permalink
fix: eleventy collection loops no longer crash
Browse files Browse the repository at this point in the history
The forloop.name feature implemented in liquidjs has now
made it to Eleventy, so the Bookshop workaround has been removed.

The workaround was causing a crash when nested inside a collection loop.
  • Loading branch information
bglw committed Jun 17, 2022
1 parent 9dfa585 commit 84a64c4
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 26 deletions.
1 change: 1 addition & 0 deletions javascript-modules/engines/eleventy-engine/lib/engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ export class Engine {
}
str = str.replace(/\n/g, ''); // TODO: Are there any cases where this breaks the eval?
const ctx = new Context();
ctx.push(this.injectInfo({}));
if (Array.isArray(props)) {
props.forEach(p => ctx.push(p));
} else {
Expand Down
1 change: 1 addition & 0 deletions javascript-modules/engines/jekyll-engine/lib/engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ export class Engine {
}
str = str.replace(/\n/g, ''); // TODO: Are there any cases where this breaks the eval?
const ctx = new Context();
ctx.push(this.injectInfo({}));
if (Array.isArray(props)) {
props.forEach(p => ctx.push(p));
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,6 @@ const getIncludeKey = (name) => {
return `shared/eleventy/${name}.eleventy.liquid`;
}

// TODO: Use forloop.name once 11ty uses liquidjs >=9.28.0
const contextHunt = (ctx, hash, index) => {
let h = stringify(hash);
for (let [k, v] of Object.entries(ctx.getAll())) {
if (!Array.isArray(v)) continue;
if (stringify(v[index]) === h) {
return k;
}
}
return "UNKNOWN";
}

module.exports = (tagType, locations, baseLocation, bookshopConfig) => (liquidEngine) => {
return {
parse: function (tagToken, remainingTokens) {
Expand Down Expand Up @@ -69,12 +57,9 @@ module.exports = (tagType, locations, baseLocation, bookshopConfig) => (liquidEn
const top_context = stack[stack.length - 1] || {};
let loop_context = '';
if (top_context["forloop"]) {
const variable = Object.keys(top_context).filter(k => k !== 'forloop')[0];

// TODO: Find the actual source. This is a guess.
const index = top_context["forloop"].index0();
const guessedSource = contextHunt(ctx, top_context[variable], index);
loop_context = `${variable}: ${guessedSource}[${index}]`;
let name = top_context["forloop"]?.name;
let index = top_context["forloop"]?.index0?.();
loop_context = `${name}[${index}]`.replace(/-/g, ': ');
}

preComment = `<!--bookshop-live name(${component}) params(${this.args}) context(${loop_context}) -->`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,11 @@ Feature: Eleventy Bookshop CloudCannon Live Editing Site Data
"""
Given a component-lib/components/block/block.eleventy.liquid file containing:
"""
<h1>{% if show %}{% for cat in collections.cat %}{% bookshop "cat" bind: cat %}{% endfor %}{% endif %}</h1>
<h1>{% if show %}{% for cat in collections.cat %}{% bookshop "cat" bind: cat.data %}{% endfor %}{% endif %}</h1>
"""
Given a component-lib/components/cat/cat.eleventy.liquid file containing:
"""
<li>{{ data.name }} ({{ data.status }})</li>
<li>{{ name }} ({{ status }})</li>
"""
* 🌐 I have loaded my site in CloudCannon
When 🌐 CloudCannon pushes new yaml:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,36 @@ Feature: Eleventy Bookshop CloudCannon Live Editing Edge Cases
Then 🌐 There should be no errors
* 🌐 There should be no logs
* 🌐 The selector p:nth-of-type(1) should contain "Item One"
* 🌐 The selector p:nth-of-type(2) should contain "Item Three"
* 🌐 The selector p:nth-of-type(2) should contain "Item Three"

# https://github.com/CloudCannon/bookshop/issues/79
Scenario: Bookshop live renders 11ty collection loop
Given a component-lib/components/tag/tag.eleventy.liquid file containing:
"""
<p>{{ name }}</p>
"""
Given a site/collection/file.md file containing:
"""
---
tags: 'collection'
layout: 'layouts/default.liquid'
name: Hey World
---
"""
Given [front_matter]:
"""
layout: layouts/default.liquid
"""
And a site/index.html file containing:
"""
---
[front_matter]
---
{% for item in collections.collection %}
{% bookshop "tag" bind: item.data %}
{% endfor %}
"""
And 🌐 I have loaded my site in CloudCannon
Then 🌐 There should be no errors
* 🌐 There should be no logs
* 🌐 The selector p should contain "Hey World"
13 changes: 10 additions & 3 deletions javascript-modules/live/lib/app/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,28 @@ if (typeof BOOKSHOP_VERSION !== "undefined") {
* Try find an existing absolute path to the given identifier,
* and note down the sum absolute path of the new name if we can
*/
export const storeResolvedPath = (name, identifier, pathStack) => {
export const storeResolvedPath = (name, identifier, pathStack, logger) => {
if (typeof identifier !== 'string') return;
// TODO: The `include.` replacement feels too SSG coupled.
// v v v v v v v v v v v v
const splitIdentifier = identifier.replace(/^include\./, '').replace(/\[(\d+)]/g, '.$1').split('.');
logger?.log?.(`Split ${identifier} info ${JSON.stringify(splitIdentifier)}`);
const baseIdentifier = splitIdentifier.shift();
logger?.log?.(`Using base identifier ${baseIdentifier}`);
if (baseIdentifier) {
const existingPath = findInStack(baseIdentifier, pathStack);
logger?.log?.(`Found the existing path ${existingPath}`);
const prefix = existingPath ?? baseIdentifier;
logger?.log?.(`Using the prefix ${prefix}`);
pathStack[pathStack.length - 1][name] = `${[prefix, ...splitIdentifier].join('.')}`;
} else {
const existingPath = findInStack(identifier, pathStack);
logger?.log?.(`Found the existing path ${existingPath}`);
const path = existingPath ?? identifier;
logger?.log?.(`Using the path ${path}`);
pathStack[pathStack.length - 1][name] = path;
}
logger?.log?.(`Stored ${name}: ${pathStack[pathStack.length - 1][name]}`);
}

// TODO: This is now partially coupled with Hugo.
Expand Down Expand Up @@ -141,10 +148,10 @@ const evaluateTemplate = async (liveInstance, documentNode, parentPathStack, tem
const normalizedIdentifier = liveInstance.normalize(identifier, logger?.nested?.());
if (typeof normalizedIdentifier === 'object' && !Array.isArray(normalizedIdentifier)) {
Object.values(normalizedIdentifier).forEach(value => {
return storeResolvedPath(name, value, pathStack)
return storeResolvedPath(name, value, pathStack, logger?.nested?.())
});
} else {
storeResolvedPath(name, normalizedIdentifier, pathStack);
storeResolvedPath(name, normalizedIdentifier, pathStack, logger?.nested?.());
}
}

Expand Down
6 changes: 4 additions & 2 deletions javascript-modules/live/lib/app/live.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,10 @@ export const getLive = (engines) => class BookshopLive {
}

async eval(identifier, scope, logger) {
logger?.log?.(`Evaluating ${identifier}`);
return await this.engines[0].eval(identifier, scope);
logger?.log?.(`Evaluating ${identifier} in ${JSON.stringify(scope)}`);
const result = await this.engines[0].eval(identifier, scope);
logger?.log?.(`Evaluated to ${result}`);
return result;
}

normalize(identifier, logger) {
Expand Down

0 comments on commit 84a64c4

Please sign in to comment.