Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw an error if a circular layout chain is detected #2076

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/TemplateLayout.js
Expand Up @@ -70,11 +70,25 @@ class TemplateLayout extends TemplateContent {
let cfgKey = this.config.keys.layout;
let map = [];
let mapEntry = await this.getTemplateLayoutMapEntry();

// Keep track of every layout we see so we can detect cyclical layout chains (e.g., a => b => c => a).
const seenLayoutKeys = new Set();
seenLayoutKeys.add(mapEntry.key);
map.push(mapEntry);

while (mapEntry.frontMatterData && cfgKey in mapEntry.frontMatterData) {
// Layout of the current layout
const parentLayoutKey = mapEntry.frontMatterData[cfgKey];
// Abort if a circular layout chain is detected. Otherwise, we'll time out and run out of memory.
if (seenLayoutKeys.has(parentLayoutKey)) {
throw new Error(
`Circular layout chain detected starting at ${map[0].key}. The layout ${parentLayoutKey} was specified twice in this layout chain.`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't sure what error message to show. It might be helpful to show the full layout chain stack. Is there a way to do that? JSON.stringify?

);
}
// Keep track of this layout so we can detect duplicates in subsequent iterations
seenLayoutKeys.add(parentLayoutKey);
let layout = TemplateLayout.getTemplate(
mapEntry.frontMatterData[cfgKey],
parentLayoutKey,
Comment on lines 70 to +91
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was getTemplateLayoutMap the right place to handle this logic, or is there a better method to put it in?

mapEntry.inputDir,
this.config,
this.extensionMap
Expand Down
22 changes: 22 additions & 0 deletions test/TemplateLayoutTest.js
Expand Up @@ -55,6 +55,28 @@ test("Get Layout Chain", async (t) => {
]);
});

test("Throw an error if a layout references itself as the layout", async (t) => {
await t.throwsAsync(async () => {
const tl = getTemplateLayoutInstance(
"layouts/layout-cycle-self.njk",
"./test/stubs"
);
const layoutChain = await tl._testGetLayoutChain();
return layoutChain;
});
});

test("Throw an error if a circular layout chain is detected", async (t) => {
await t.throwsAsync(async () => {
const tl = getTemplateLayoutInstance(
"layouts/layout-cycle-a.njk",
"./test/stubs"
);
const layoutChain = await tl._testGetLayoutChain();
return layoutChain;
});
});

test("Get Front Matter Data", async (t) => {
let tl = getTemplateLayoutInstance(
"layouts/layout-inherit-a.njk",
Expand Down
3 changes: 3 additions & 0 deletions test/stubs/_includes/layouts/layout-cycle-a.njk
@@ -0,0 +1,3 @@
---
layout: layouts/layout-cycle-b.njk
---
3 changes: 3 additions & 0 deletions test/stubs/_includes/layouts/layout-cycle-b.njk
@@ -0,0 +1,3 @@
---
layout: layouts/layout-cycle-c.njk
---
3 changes: 3 additions & 0 deletions test/stubs/_includes/layouts/layout-cycle-c.njk
@@ -0,0 +1,3 @@
---
layout: layouts/layout-cycle-a.njk
---
3 changes: 3 additions & 0 deletions test/stubs/_includes/layouts/layout-cycle-self.njk
@@ -0,0 +1,3 @@
---
layout: layouts/layout-cycle-self.njk
---