From 84a64c4cb74243d3030373f8f9ddac68c48dfd75 Mon Sep 17 00:00:00 2001 From: Liam Bigelow <40188355+bglw@users.noreply.github.com> Date: Fri, 17 Jun 2022 12:46:14 +1200 Subject: [PATCH] fix: eleventy collection loops no longer crash 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. --- .../engines/eleventy-engine/lib/engine.js | 1 + .../engines/jekyll-engine/lib/engine.js | 1 + .../lib/eleventy-one-bookshop.js | 21 ++---------- .../eleventy_bookshop_live_data.feature | 4 +-- .../eleventy_bookshop_live_edge_cases.feature | 34 ++++++++++++++++++- javascript-modules/live/lib/app/core.js | 13 +++++-- javascript-modules/live/lib/app/live.js | 6 ++-- 7 files changed, 54 insertions(+), 26 deletions(-) diff --git a/javascript-modules/engines/eleventy-engine/lib/engine.js b/javascript-modules/engines/eleventy-engine/lib/engine.js index 211330ea..71c76533 100644 --- a/javascript-modules/engines/eleventy-engine/lib/engine.js +++ b/javascript-modules/engines/eleventy-engine/lib/engine.js @@ -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 { diff --git a/javascript-modules/engines/jekyll-engine/lib/engine.js b/javascript-modules/engines/jekyll-engine/lib/engine.js index f8b4b393..89589376 100644 --- a/javascript-modules/engines/jekyll-engine/lib/engine.js +++ b/javascript-modules/engines/jekyll-engine/lib/engine.js @@ -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 { diff --git a/javascript-modules/generator-plugins/eleventy/eleventy-bookshop/lib/eleventy-one-bookshop.js b/javascript-modules/generator-plugins/eleventy/eleventy-bookshop/lib/eleventy-one-bookshop.js index 08ff89a6..70d97014 100644 --- a/javascript-modules/generator-plugins/eleventy/eleventy-bookshop/lib/eleventy-one-bookshop.js +++ b/javascript-modules/generator-plugins/eleventy/eleventy-bookshop/lib/eleventy-one-bookshop.js @@ -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) { @@ -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 = ``; diff --git a/javascript-modules/integration-tests/features/eleventy/live_editing/eleventy_bookshop_live_data.feature b/javascript-modules/integration-tests/features/eleventy/live_editing/eleventy_bookshop_live_data.feature index 190d8b96..285eaa8b 100644 --- a/javascript-modules/integration-tests/features/eleventy/live_editing/eleventy_bookshop_live_data.feature +++ b/javascript-modules/integration-tests/features/eleventy/live_editing/eleventy_bookshop_live_data.feature @@ -116,11 +116,11 @@ Feature: Eleventy Bookshop CloudCannon Live Editing Site Data """ Given a component-lib/components/block/block.eleventy.liquid file containing: """ -
{{ name }}
+ """ + 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" diff --git a/javascript-modules/live/lib/app/core.js b/javascript-modules/live/lib/app/core.js index 8f83f217..ae84c212 100644 --- a/javascript-modules/live/lib/app/core.js +++ b/javascript-modules/live/lib/app/core.js @@ -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. @@ -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?.()); } } diff --git a/javascript-modules/live/lib/app/live.js b/javascript-modules/live/lib/app/live.js index ef25e87e..18065160 100644 --- a/javascript-modules/live/lib/app/live.js +++ b/javascript-modules/live/lib/app/live.js @@ -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) {