Skip to content

Commit

Permalink
A bunch of fixes to Paginated Data via Proxy approach from ceb5e70
Browse files Browse the repository at this point in the history
  • Loading branch information
zachleat committed May 6, 2022
1 parent ceb5e70 commit c6158b0
Show file tree
Hide file tree
Showing 21 changed files with 382 additions and 58 deletions.
34 changes: 33 additions & 1 deletion src/ComputedData.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class ComputedData {
this.computedKeys = new Set();
this.declaredDependencies = {};
this.queue = new ComputedDataQueue();
this.usesLog = {};
this.config = config;
}

Expand All @@ -25,6 +26,7 @@ class ComputedData {
// bind config filters/JS functions
if (typeof renderFn === "function") {
let fns = {};
// TODO bug? no access to non-universal config things?
if (this.config) {
fns = this.config.javascriptFunctions;
}
Expand Down Expand Up @@ -76,6 +78,9 @@ class ComputedData {
varsUsed = await proxy.findVarsUsed(computed, data);
}

// save the uses for hoisting below
this.usesLog[key] = varsUsed;

debug("%o accesses %o variables", key, varsUsed);
let filteredVarsUsed = varsUsed.filter((varUsed) => {
return (
Expand All @@ -88,12 +93,39 @@ class ComputedData {
}
}

// Workaround to hoist proxied data (via pagination) into the top level object for use in template languages
// (see tests: "Pagination over collection using eleventyComputed")
hoistProxyData(data, vars = []) {
// pagination is the only code path using proxies for data aliasing
if (!("pagination" in data)) {
return;
}
let defaultValue = "____DEFAULT_11ty_VALUE___";
for (let v of vars) {
let value = lodashGet(data, v, defaultValue);
if (value !== defaultValue) {
lodashSet(data, v, value);
}
}
}

getHoistedVars(key) {
let deps = this.usesLog[key];
let hoists = Array.from(new Set(deps));
return hoists;
}

async _setupDataEntry(data, order) {
debug("Computed data order of execution: %o", order);

for (let key of order) {
let computed = lodashGet(this.computed, key);

if (typeof computed === "function") {
let shouldHoistData = !!this.templateStringKeyLookup[key];
if (shouldHoistData) {
let hoists = this.getHoistedVars(key);
this.hoistProxyData(data, hoists);
}
let ret = await computed(data);
lodashSet(data, key, ret);
} else if (computed !== undefined) {
Expand Down
2 changes: 2 additions & 0 deletions src/ComputedDataProxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ class ComputedDataProxy {
}

getProxyData(data, keyRef) {
// WARNING: SIDE EFFECTS
// TODO should make another effort to get rid of this
// Set defaults for keys not already set on parent data
let undefinedValue = "__11TY_UNDEFINED__";
if (this.computedKeys) {
Expand Down
3 changes: 2 additions & 1 deletion src/Engines/Nunjucks.js
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,8 @@ class Nunjucks extends TemplateEngine {
return name.join(".");
});

return Array.from(new Set(symbols));
let uniqueSymbols = Array.from(new Set(symbols));
return uniqueSymbols;
}

async compile(str, inputPath) {
Expand Down
51 changes: 27 additions & 24 deletions src/Plugins/Pagination.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,7 @@ const lodashGet = require("lodash/get");
const lodashSet = require("lodash/set");
const EleventyBaseError = require("../EleventyBaseError");
const { DeepCopy } = require("../Util/Merge");

function proxyWrap(target, fallback) {
return new Proxy(target, {
get(target, prop) {
if (prop in target) {
return target[prop];
}
return fallback[prop];
},
});
}
const { ProxyWrap } = require("../Util/ProxyWrap");

class PaginationConfigError extends EleventyBaseError {}
class PaginationError extends EleventyBaseError {}
Expand Down Expand Up @@ -306,21 +296,36 @@ class Pagination {
let links = [];
let hrefs = [];

let paginationDataObject = {
data: this.data.pagination.data,
size: this.data.pagination.size,
alias: this.alias,
pages,
};

let hasPermalinkField = Boolean(this.data[this.config.keys.permalink]);
let hasComputedPermalinkField = Boolean(
this.data.eleventyComputed &&
this.data.eleventyComputed[this.config.keys.permalink]
);

// Reuse parent `template.dataCache` for a small memory efficiency win
let parentData = DeepCopy({}, this.data);
// Reuse parent `template.data` cache for a small memory efficiency win
let collections = this.data.collections;
if (collections) {
delete this.data.collections;
}

let parentData = DeepCopy(
{
pagination: {
data: this.data.pagination.data,
size: this.data.pagination.size,
alias: this.alias,
pages,
},
},
this.data
);

// Restore skipped collections
if (collections) {
this.data.collections = collections;
// Keep the original reference to the collections, no deep copy!!
parentData.collections = collections;
}

// TODO future improvement dea: use a light Template wrapper for paged template clones (PagedTemplate?)
// so that we don’t have the memory cost of the full template (and can reuse the parent
Expand All @@ -340,10 +345,10 @@ class Pagination {
pagination: {
items: items[pageNumber],
},
page: {},
};
Object.assign(
paginationData.pagination,
paginationDataObject,
this.getOverrideDataPages(items, pageNumber)
);

Expand All @@ -355,10 +360,8 @@ class Pagination {
);
}

paginationData.page = proxyWrap({}, parentData.page);

// Do *not* deep merge pagination data! See https://github.com/11ty/eleventy/issues/147#issuecomment-440802454
let clonedData = proxyWrap(paginationData, parentData);
let clonedData = ProxyWrap(paginationData, parentData);

let { rawPath, path, href } = await cloned.getOutputLocations(clonedData);
// TO DO subdirectory to links if the site doesn’t live at /
Expand Down
17 changes: 4 additions & 13 deletions src/Template.js
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ class Template extends TemplateContent {
computedData.addTemplateString(
parentKey,
async (innerData) => {
return await this.renderComputedData(obj, innerData, true);
return await this.renderComputedData(obj, innerData);
},
declaredDependencies,
this.getParseForSymbolsFunction(obj)
Expand All @@ -564,6 +564,8 @@ class Template extends TemplateContent {
// Note that `permalink` is only a thing that gets consumed—it does not go directly into generated data
// this allows computed entries to use page.url or page.outputPath and they’ll be resolved properly

// TODO Room for optimization here—we don’t need to recalculate `getOutputHref` and `getOutputPath`
// TODO Why are these using addTemplateString instead of add
this.computedData.addTemplateString(
"page.url",
async (data) => await this.getOutputHref(data),
Expand Down Expand Up @@ -618,9 +620,7 @@ class Template extends TemplateContent {

async getTemplates(data) {
// no pagination with permalink.serverless
let hasPagination = Pagination.hasPagination(data);

if (!hasPagination) {
if (!Pagination.hasPagination(data)) {
await this.addComputedData(data);

return [
Expand Down Expand Up @@ -669,15 +669,6 @@ class Template extends TemplateContent {
let pageTemplates = await this.paging.getPageTemplates();
return await Promise.all(
pageTemplates.map(async (pageEntry, pageNumber) => {
if (data.collections) {
// Issue #115
pageEntry.data.collections = data.collections;
}

// eleventyComputed needs to exist on proxy for it to iterate and set data
if (data.eleventyComputed) {
pageEntry.data.eleventyComputed = data.eleventyComputed;
}
await pageEntry.template.addComputedData(pageEntry.data);

return {
Expand Down
18 changes: 16 additions & 2 deletions src/TemplateContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -321,9 +321,21 @@ class TemplateContent {
}

getParseForSymbolsFunction(str) {
if ("parseForSymbols" in this.engine) {
let engine = this.engine;

// Don’t use markdown as the engine to parse for symbols
let preprocessorEngine = this.templateRender.getPreprocessorEngine(); // TODO pass in engineOverride here
if (preprocessorEngine && engine.getName() !== preprocessorEngine) {
let replacementEngine =
this.templateRender.getEngineByName(preprocessorEngine);
if (replacementEngine) {
engine = replacementEngine;
}
}

if ("parseForSymbols" in engine) {
return () => {
return this.engine.parseForSymbols(str);
return engine.parseForSymbols(str);
};
}
}
Expand Down Expand Up @@ -360,6 +372,7 @@ class TemplateContent {
.incrementCount();

let permalinkCompilation = this.engine.permalinkNeedsCompilation(permalink);

// No string compilation:
// ({ compileOptions: { permalink: "raw" }})
// These mean `permalink: false`, which is no file system writing:
Expand Down Expand Up @@ -426,6 +439,7 @@ class TemplateContent {
bypassMarkdown,
data[this.config.keys.engineOverride]
);

if (fn === undefined) {
return;
} else if (typeof fn !== "function") {
Expand Down
29 changes: 23 additions & 6 deletions src/TemplateRender.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,17 @@ class TemplateRender {
return this._extensionMap;
}

getEngineByName(name) {
let engine = this.extensionMap.engineManager.getEngine(
name,
this.getDirs(),
this.extensionMap
);
engine.config = this.config;

return engine;
}

// Runs once per template
init(engineNameOrPath) {
this.extensionMap.config = this.config;
Expand All @@ -68,12 +79,7 @@ class TemplateRender {
);
}

this._engine = this.extensionMap.engineManager.getEngine(
this._engineName,
this.getDirs(),
this.extensionMap
);
this._engine.config = this.config;
this._engine = this.getEngineByName(this._engineName);
this._engine.initRequireCache(engineNameOrPath);

if (this.useMarkdown === undefined) {
Expand Down Expand Up @@ -172,6 +178,17 @@ class TemplateRender {
}
}

// TODO templateEngineOverride
getPreprocessorEngine() {
if (this.engineName === "md" && this.parseMarkdownWith) {
return this.parseMarkdownWith;
}
if (this.engineName === "html" && this.parseHtmlWith) {
return this.parseHtmlWith;
}
return this.extensionMap.getKey(this.engineNameOrPath);
}

// We pass in templateEngineOverride here because it isn’t yet applied to templateRender
getEnginesList(engineOverride) {
if (engineOverride) {
Expand Down
31 changes: 20 additions & 11 deletions src/Util/Merge.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
const { isPlainObject } = require("@11ty/eleventy-utils");
const OVERRIDE_PREFIX = "override:";

function getMergedItem(target, source, parentKey, overridePrefix) {
// if key is prefixed with overridePrefix, it just keeps the new source value (no merging)
if (overridePrefix && parentKey && parentKey.indexOf(overridePrefix) === 0) {
function cleanKey(key, prefix) {
if (prefix && key.startsWith(prefix)) {
return key.substr(prefix.length);
}
return key;
}

function getMergedItem(target, source, parentKey, prefixes = {}) {
let { override } = prefixes;

// if key is prefixed with override prefix, it just keeps the new source value (no merging)
if (override && parentKey && parentKey.startsWith(override)) {
return source;
}

Expand All @@ -17,15 +26,13 @@ function getMergedItem(target, source, parentKey, overridePrefix) {
} else if (isPlainObject(target)) {
if (isPlainObject(source)) {
for (let key in source) {
let newKey = key;
if (overridePrefix && key.startsWith(overridePrefix)) {
newKey = key.substr(overridePrefix.length);
}
target[newKey] = getMergedItem(
let overrideKey = cleanKey(key, override);

target[overrideKey] = getMergedItem(
target[key],
source[key],
newKey,
overridePrefix
overrideKey,
prefixes
);
}
}
Expand Down Expand Up @@ -63,7 +70,9 @@ function Merge(target, ...sources) {
if (!source) {
continue;
}
target = getMergedItem(target, source, null, OVERRIDE_PREFIX);
target = getMergedItem(target, source, null, {
override: OVERRIDE_PREFIX,
});
}

return target;
Expand Down

0 comments on commit c6158b0

Please sign in to comment.