Skip to content

Commit

Permalink
fix: handle some editor link corner cases
Browse files Browse the repository at this point in the history 
* fix: wait for any bookshop data to resolve before live rendering

* feat: add an editor link flag on a per-component basis

* fix: don't add editor links to shared includes by default
  • Loading branch information
@bglw
bglw authored Dec 7, 2021
1 parent 3189781 commit a45653a
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 42 deletions.
6 changes: 6 additions & 0 deletions javascript-modules/engines/eleventy-engine/lib/engine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ export class Engine {
return !!this.files?.[key];
}

resolveComponentType(name) {
if (this.getComponent(name)) return 'component';
if (this.getShared(name)) return 'shared';
return false;
}

async render(target, name, props, globals) {
let source = this.getComponent(name);
// TODO: Remove the below check and update the live comments to denote shared
Expand Down
6 changes: 6 additions & 0 deletions javascript-modules/engines/jekyll-engine/lib/engine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ export class Engine {
return !!this.files?.[key];
}

resolveComponentType(name) {
if (this.getComponent(name)) return 'component';
if (this.getShared(name)) return 'shared';
return false;
}

async render(target, name, props, globals) {
let source = this.getComponent(name);
// TODO: Remove the below check and update the live comments to denote shared
Expand Down
49 changes: 27 additions & 22 deletions javascript-modules/live/lib/app/core.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ export const renderComponentUpdates = async (liveInstance, documentNode) => {
bindings,
output
)
updates.push({ startNode, endNode, output, pathStack });
updates.push({ startNode, endNode, output, pathStack, scope, name });
}

await evaluateTemplate(liveInstance, documentNode, null, templateBlockHandler);
Expand Down Expand Up @@ -190,30 +190,35 @@ export const hydrateEditorLinks = async (liveInstance, documentNode, pathsInScop
components.push(component);
}

// pathsInScope are from an earlier render pass, so will contain any paths
// at a higher level than the documentNode we're working on. Without this,
// if documentNode is a subcomponent we wouldn't be able to know
// its path back to the root data.
await evaluateTemplate(liveInstance, documentNode, pathsInScope, templateBlockHandler);

for (let { startNode, endNode, params, pathStack } of components) {
// pathsInScope are from an earlier render pass, so will contain any paths
// at a higher level than the documentNode we're working on. Without this,
// if documentNode is a subcomponent we wouldn't be able to know
// its path back to the root data.
let editorLink = null;
for (const [, identifier] of params) {
const path = (findInStack(identifier, pathStack) ?? identifier);
let pathResolves = dig(liveInstance.data, path);
if (pathResolves) {
// TODO: This special page case feels too SSG-coupled
editorLink = path.replace(/^page(\.|$)/, '');
break;
for (let { startNode, endNode, params, pathStack, scope, name } of components) {
// By default, don't add editor links for bookshop shared includes
const isStandardComponent = liveInstance.resolveComponentType(name) === 'component';
const editorLinkFlag = scope?.editorLink ?? scope?.editor_link ?? isStandardComponent;
if (editorLinkFlag) { // If we should be adding an editor link _for this component_
let editorLink = null;
for (const [, identifier] of params) {
const path = (findInStack(identifier, pathStack) ?? identifier);
let pathResolves = dig(liveInstance.data, path);
if (pathResolves) {
// TODO: This special page case feels too SSG-coupled
editorLink = path.replace(/^page(\.|$)/, '');
break;
}
}
}
if (editorLink) {
// Add the editor link to all top-level elements of the component,
// since we can't wrap the component in any elements
let node = startNode.nextElementSibling;
while (node && (node.compareDocumentPosition(endNode) & Node.DOCUMENT_POSITION_FOLLOWING) !== 0) {
node.dataset.cmsEditorLink = `cloudcannon:#${editorLink}`;
node = node.nextElementSibling;
if (editorLink) {
// Add the editor link to all top-level elements of the component,
// since we can't wrap the component in any elements
let node = startNode.nextElementSibling;
while (node && (node.compareDocumentPosition(endNode) & Node.DOCUMENT_POSITION_FOLLOWING) !== 0) {
node.dataset.cmsEditorLink = `cloudcannon:#${editorLink}`;
node = node.nextElementSibling;
}
}
}
}
Expand Down
39 changes: 30 additions & 9 deletions javascript-modules/live/lib/app/live.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,25 @@ export const getLive = (engines) => class BookshopLive {
this.engines = engines;
this.elements = [];
this.globalData = {};
this.data = {};
this.renderOptions = {};
this.pendingRender = false;
this.awaitingDataFetches = options?.remoteGlobals?.length || 0;
options?.remoteGlobals?.forEach(this.fetchGlobalData.bind(this));
}

async fetchGlobalData(path) {
const dataReq = await fetch(path);
const data = await dataReq.json();
Object.assign(this.globalData, data);
try {
const dataReq = await fetch(path);
const data = await dataReq.json();
Object.assign(this.globalData, data);
this.awaitingDataFetches -= 1;
} catch (e) {
this.awaitingDataFetches -= 1;
}
if (this.awaitingDataFetches <= 0 && this.pendingRender) {
await this.render()
}
}

readElement(el) {
Expand All @@ -23,6 +35,10 @@ export const getLive = (engines) => class BookshopLive {
}
}

resolveComponentType(componentName) {
return this.engines[0].resolveComponentType(componentName);
}

async renderElement(componentName, scope, bindings, dom) {
try {
await this.engines[0].render(dom, componentName, scope, { ...this.globalData, ...bindings });
Expand All @@ -38,14 +54,19 @@ export const getLive = (engines) => class BookshopLive {

async update(data, options) {
this.data = data;
await this.render(options);
this.renderOptions = options;
if (this.awaitingDataFetches > 0) {
this.pendingRender = true;
} else {
await this.render();
}
}

async render(options = {}) {
const CCEditorPanelSupport = typeof window !== 'undefined' && window.CloudCannon?.refreshInterface;
options = {
async render() {
const CCEditorPanelSupport = typeof window === 'undefined' || typeof window !== 'undefined' && window.CloudCannon?.refreshInterface;
const options = {
editorLinks: CCEditorPanelSupport,
...options
...this.renderOptions
};

// Render _all_ components found on the page into virtual DOM nodes
Expand All @@ -59,7 +80,7 @@ export const getLive = (engines) => class BookshopLive {
output, // A virtual-DOM node containing contents of the just-rendered component
pathStack, // Any "absolute paths" to data in scope for this component
} of componentUpdates) {
if (options.editorLinks) {
if (options.editorLinks) { // If we should be adding editor links _in general_
// Re-traverse this component to inject any editor links we can to it or its children.
await core.hydrateEditorLinks(this, output, pathStack, startNode.cloneNode(), endNode.cloneNode());
}
Expand Down
57 changes: 46 additions & 11 deletions javascript-modules/live/lib/app/live.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,35 +14,39 @@ const jsdomWindow = (new JSDOM(`...`)).window;
});

const jekyllComponent = (k) => `components/${k}/${k}.jekyll.html`;
const jekyllShared = (k) => `shared/jekyll/${k}.jekyll.html`;
const jekyllFiles = {
[jekyllComponent('title')]: "<h1>{{ include.title }}</h1>",
[jekyllComponent('null')]: "<null>empty</null>",
[jekyllComponent('super-title')]: "{% bookshop title title=include.one %}<h1>{{ include.two }}</h2>{% bookshop title title=include.three %}",
[jekyllComponent('super-wrapped-title-inner')]: "<div>{% bookshop title title=include.one %}</div><h1>{{ include.two }}</h2><div>{% bookshop title title=include.three %}</div>",
[jekyllComponent('super-wrapped-title')]: "<div>{% bookshop super-wrapped-title-inner bind=include.inner %}</div>",
[jekyllComponent('title-loop')]: "<div>{% for t in include.titles %}{% bookshop title title=t %}{% endfor %}</div>",
[jekyllShared('title-loop')]: "{% for t in include.titles %}{% bookshop title title=t %}{% endfor %}",
[jekyllComponent('num-loop')]: "{% for t in (include.min..include.max) %}{% bookshop title title=t %}{% endfor %}",
[jekyllComponent('wrapper')]: "{% bookshop {{include.component}} bind=page.data %}",
[jekyllComponent('include-wrapper')]: "{% bookshop_include {{include.component}} bind=page.data %}",
[jekyllComponent('title-wrapper')]: "{% bookshop {{include.component}} title=page.title %}",
[jekyllComponent('titles-wrapper')]: "{% bookshop {{include.component}} titles=page.titles %}",
[jekyllComponent('titles-wrapper-erroneous-component')]: "{% bookshop {{include.component}} titles=page.titles %}{% bookshop null null=t %}",
[jekyllComponent('titles-wrapper')]: "{% bookshop_include {{include.component}} titles=page.titles %}",
[jekyllComponent('titles-wrapper-erroneous-component')]: "{% bookshop_include {{include.component}} titles=page.titles %}{% bookshop null null=t %}",
[jekyllComponent('assign-wrapper')]: "{% assign test_var=include.component %}{% bookshop {{test_var}} bind=page %}",
[jekyllComponent('dynamic-loop')]: "{% for props in include.t %}{% bookshop {{props._bookshop_name}} bind=props %}{% endfor %}",
}

const eleventyComponent = (k) => `components/${k}/${k}.eleventy.liquid`;
const eleventyShared = (k) => `shared/eleventy/${k}.eleventy.liquid`;
const eleventyFiles = {
[eleventyComponent('title')]: "<h1>{{ title }}</h1>",
[eleventyComponent('null')]: "<null>empty</null>",
[eleventyComponent('super-title')]: "{% bookshop 'title' title: one %}<h1>{{ two }}</h2>{% bookshop 'title' title: three %}",
[eleventyComponent('super-wrapped-title-inner')]: "<div>{% bookshop 'title' title: one %}</div><h1>{{ two }}</h2><div>{% bookshop 'title' title: three %}</div>",
[eleventyComponent('super-wrapped-title')]: "<div>{% bookshop 'super-wrapped-title-inner' bind: inner %}</div>",
[eleventyComponent('title-loop')]: "<div>{% for t in titles %}{% bookshop 'title' title: t %}{% endfor %}</div>",
[eleventyShared('title-loop')]: "{% for t in titles %}{% bookshop 'title' title: t %}{% endfor %}",
[eleventyComponent('num-loop')]: "{% for t in (min..max) %}{% bookshop 'title' title: t %}{% endfor %}",
[eleventyComponent('wrapper')]: "{% bookshop '{{component}}' bind: data %}",
[eleventyComponent('include-wrapper')]: "{% bookshop_include '{{component}}' bind: data %}",
[eleventyComponent('title-wrapper')]: "{% bookshop '{{component}}' title: title %}",
[eleventyComponent('titles-wrapper')]: "{% bookshop '{{component}}' titles: titles %}",
[eleventyComponent('titles-wrapper-erroneous-component')]: "{% bookshop '{{component}}' titles: titles %}{% bookshop 'null' null: t %}",
[eleventyComponent('titles-wrapper')]: "{% bookshop_include '{{component}}' titles: titles %}",
[eleventyComponent('titles-wrapper-erroneous-component')]: "{% bookshop_include '{{component}}' titles: titles %}{% bookshop 'null' null: t %}",
[eleventyComponent('assign-wrapper')]: "{% assign test_var=component %}{% bookshop '{{test_var}}' bind: page %}",
[eleventyComponent('dynamic-loop')]: "{% for props in t %}{% bookshop {{props._bookshop_name}} bind: props %}{% endfor %}",
}
Expand Down Expand Up @@ -103,7 +107,7 @@ for (const impl of ['jekyll', 'eleventy']) {
});

test.serial(`[${impl}]: Re-renders in a loop`, async t => {
await initialSub(t.context[impl], 'title-loop', { data: { titles: ['Bookshop', 'Jekyll', 'Eleventy'] } });
await initialSub(t.context[impl], 'title-loop', { data: { titles: ['Bookshop', 'Jekyll', 'Eleventy'] } }, 'include-wrapper');
t.regex(getBody(), /<h1>Jekyll<\/h1>/);

let trigger = false;
Expand Down Expand Up @@ -210,7 +214,7 @@ for (const impl of ['jekyll', 'eleventy']) {
]
}
]
})
}, { editorLinks: false })
t.regex(getBody(), /<h1>First Changed Inner Hello World<\/h1>/);
t.notRegex(getBody(), /<h1>First Inner Hello World<\/h1>/);
t.regex(getBody(), /<h1>Third Inner Hello World<\/h1>/);
Expand Down Expand Up @@ -257,7 +261,7 @@ for (const impl of ['jekyll', 'eleventy']) {
await initialSub(t.context[impl], 'super-title', { data: { one: "One", two: "Two", three: "Three" } });

const data = wrapDataFor(impl, { data: { one: "One", two: "Two", three: "Three" } });
await t.context[impl].update(data, { editorLinks: true })
await t.context[impl].update(data)
t.regex(getBody(), /<h1 data-cms-editor-link="cloudcannon:#data">One<\/h1>/);
t.regex(getBody(), /<h1 data-cms-editor-link="cloudcannon:#data">Two<\/h1>/);
t.regex(getBody(), /<h1 data-cms-editor-link="cloudcannon:#data">Three<\/h1>/);
Expand All @@ -267,8 +271,7 @@ for (const impl of ['jekyll', 'eleventy']) {
await initialSub(t.context[impl], 'title-loop', { titles: ['Bookshop', 'Jekyll', 'Eleventy'] }, 'titles-wrapper');

const data = wrapDataFor(impl, { titles: ['Bookshop', 'Jekyll', 'Eleventy'] });
await t.context[impl].update(data, { editorLinks: true })
t.regex(getBody(), /<div data-cms-editor-link="cloudcannon:#titles">/);
await t.context[impl].update(data)
t.regex(getBody(), /<h1 data-cms-editor-link="cloudcannon:#titles.0">Bookshop<\/h1>/);
t.regex(getBody(), /<h1 data-cms-editor-link="cloudcannon:#titles.1">Jekyll<\/h1>/);
t.regex(getBody(), /<h1 data-cms-editor-link="cloudcannon:#titles.2">Eleventy<\/h1>/);
Expand Down Expand Up @@ -310,4 +313,36 @@ for (const impl of ['jekyll', 'eleventy']) {
document.querySelectorAll('h1')[1].click();
t.is(trigger, true);
});

test.serial(`[${impl}]: Respects the editorLinks flag`, async t => {
await initialSub(t.context[impl], 'super-wrapped-title', { data: { editorLink: false, inner: { one: "One", two: "Two", three: "Three" } } });

// Render without the outer-most editor link
let data = wrapDataFor(impl, { data: { editorLink: false, inner: { one: "One", two: "Two", three: "Three" } } });
await t.context[impl].update(data, { editorLinks: true })

t.notRegex(getBody(), /<div data-cms-editor-link="cloudcannon:#data">/);
t.regex(getBody(), /<div data-cms-editor-link="cloudcannon:#data.inner">/);

// Render without the inner editor link
data = wrapDataFor(impl, { data: { editorLink: true, inner: { editorLink: false, one: "One", two: "Two", three: "Three" } } });
await t.context[impl].update(data, { editorLinks: true })

t.regex(getBody(), /<div data-cms-editor-link="cloudcannon:#data">/);
t.notRegex(getBody(), /<div data-cms-editor-link="cloudcannon:#data.inner">/);

// Render with both editor links
data = wrapDataFor(impl, { data: { inner: { one: "One", two: "Two", three: "Three" } } });
await t.context[impl].update(data, { editorLinks: true })

t.regex(getBody(), /<div data-cms-editor-link="cloudcannon:#data">/);
t.regex(getBody(), /<div data-cms-editor-link="cloudcannon:#data.inner">/);

// Render without the inner editor link using the other syntax
data = wrapDataFor(impl, { data: { editor_link: true, inner: { editor_link: false, one: "One", two: "Two", three: "Three" } } });
await t.context[impl].update(data, { editorLinks: true })

t.regex(getBody(), /<div data-cms-editor-link="cloudcannon:#data">/);
t.notRegex(getBody(), /<div data-cms-editor-link="cloudcannon:#data.inner">/);
});
}

0 comments on commit a45653a

Please sign in to comment.