From febbea413dbfc86c1670e65b97c16edddbec1b39 Mon Sep 17 00:00:00 2001 From: Zach Leatherman Date: Wed, 24 Jul 2019 22:28:24 -0500 Subject: [PATCH 1/4] =?UTF-8?q?This=20needs=20more=20tests=20but=20it?= =?UTF-8?q?=E2=80=99s=20a=20good=20start=20at=20adding=20support=20for=20p?= =?UTF-8?q?ublic=20class=20fields,=20one=20class=20instance=20for=20data?= =?UTF-8?q?=20and=20render,=20as=20well=20as=20multiple=20exports=20as=20d?= =?UTF-8?q?escribed=20here=20https://github.com/11ty/eleventy/issues/622#i?= =?UTF-8?q?ssuecomment-514681826?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also adds travis tests for Node 12 (required for public class fields) #622 --- .travis.yml | 1 + src/Engines/JavaScript.js | 54 +++++++++++++++++++++++------ test/TemplateRenderLiquidTest.js | 1 + test/TemplateRenderNunjucksTest.js | 1 + test/TemplateTest-JavaScript.js | 45 ++++++++++++++++++++++++ test/stubs/classfields-data.11ty.js | 11 ++++++ test/stubs/multipleexports.11ty.js | 7 ++++ test/stubs/oneinstance.11ty.js | 18 ++++++++++ 8 files changed, 127 insertions(+), 11 deletions(-) create mode 100644 test/stubs/classfields-data.11ty.js create mode 100644 test/stubs/multipleexports.11ty.js create mode 100644 test/stubs/oneinstance.11ty.js diff --git a/.travis.yml b/.travis.yml index 0daa7ca9a..4468c9a8a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,6 +2,7 @@ language: node_js node_js: - 8 - 10 + - 12 before_script: - npm install script: npm run test diff --git a/src/Engines/JavaScript.js b/src/Engines/JavaScript.js index 689e5801f..de3c0799d 100644 --- a/src/Engines/JavaScript.js +++ b/src/Engines/JavaScript.js @@ -2,6 +2,11 @@ const TemplateEngine = require("./TemplateEngine"); const TemplatePath = require("../TemplatePath"); class JavaScript extends TemplateEngine { + constructor(name, includesDir) { + super(name, includesDir); + this.instances = {}; + } + normalize(result) { if (Buffer.isBuffer(result)) { return result.toString(); @@ -10,6 +15,24 @@ class JavaScript extends TemplateEngine { return result; } + getInstanceFromInputPath(inputPath) { + if (this.instances[inputPath]) { + return this.instances[inputPath]; + } + + const cls = this._getRequire(inputPath); + if (typeof cls === "function") { + if ( + cls.prototype && + ("data" in cls.prototype || "render" in cls.prototype) + ) { + let inst = new cls(); + this.instances[inputPath] = inst; + return inst; + } + } + } + _getRequire(inputPath) { let requirePath = TemplatePath.absolutePath(inputPath); return require(requirePath); @@ -25,21 +48,24 @@ class JavaScript extends TemplateEngine { if (requirePath in require.cache) { delete require.cache[requirePath]; } + + if (inputPath in this.instances) { + delete this.instances[inputPath]; + } } async getExtraDataFromFile(inputPath) { - const cls = this._getRequire(inputPath); - if (typeof cls === "function") { - if (cls.prototype && "data" in cls.prototype) { - // TODO this is instantiating multiple separate instances every time it is called (see also one in compile) - let inst = new cls(); - // get extra data from `data` method, - // either as a function or getter or object literal - return typeof inst.data === "function" ? await inst.data() : inst.data; - } + let inst = this.getInstanceFromInputPath(inputPath); + if (inst) { + // get extra data from `data` method, + // either as a function or getter or object literal + return typeof inst.data === "function" ? await inst.data() : inst.data; } - return {}; + const cls = this._getRequire(inputPath); + if (typeof cls === "object") { + return typeof cls.data === "function" ? await cls.data() : cls.data; + } } async compile(str, inputPath) { @@ -60,7 +86,7 @@ class JavaScript extends TemplateEngine { if (typeof cls === "function") { // class with a `render` method if (cls.prototype && "render" in cls.prototype) { - let inst = new cls(); + let inst = this.getInstanceFromInputPath(inputPath); Object.assign(inst, this.config.javascriptFunctions); return function(data) { return this.normalize(inst.render.call(inst, data)); @@ -71,6 +97,12 @@ class JavaScript extends TemplateEngine { return function(data) { return this.normalize(cls.call(this.config.javascriptFunctions, data)); }.bind(this); + } else if (typeof cls === "object" && "render" in cls) { + return function(data) { + return this.normalize( + cls.render.call(this.config.javascriptFunctions, data) + ); + }.bind(this); } else { // string type does not work with javascriptFunctions return function() { diff --git a/test/TemplateRenderLiquidTest.js b/test/TemplateRenderLiquidTest.js index 2e2148e4b..0bed58f5c 100644 --- a/test/TemplateRenderLiquidTest.js +++ b/test/TemplateRenderLiquidTest.js @@ -722,6 +722,7 @@ test("Issue 600: Liquid Shortcode argument with underscores", async t => { }); test.skip("Issue 611: Run a function", async t => { + // This works in Nunjucks let tr = new TemplateRender("liquid", "./test/stubs/"); t.is( diff --git a/test/TemplateRenderNunjucksTest.js b/test/TemplateRenderNunjucksTest.js index 065dd7835..11a499038 100644 --- a/test/TemplateRenderNunjucksTest.js +++ b/test/TemplateRenderNunjucksTest.js @@ -419,6 +419,7 @@ test("Nunjucks Test if statements on arrays (Issue #524)", async t => { }); test("Issue 611: Run a function", async t => { + // This does not work in Liquid let tr = new TemplateRender("njk", "./test/stubs/"); t.is( diff --git a/test/TemplateTest-JavaScript.js b/test/TemplateTest-JavaScript.js index c556a6a5e..56e8b5347 100644 --- a/test/TemplateTest-JavaScript.js +++ b/test/TemplateTest-JavaScript.js @@ -1,5 +1,6 @@ import test from "ava"; import Template from "../src/Template"; +import semver from "semver"; test("JavaScript template type (function)", async t => { let tmpl = new Template( @@ -44,6 +45,22 @@ test("JavaScript template type (class with data method)", async t => { t.is(pages[0].templateContent.trim(), "

Ted

"); }); +if (semver.gte(process.version, "12.4.0")) { + test("JavaScript template type (class fields)", async t => { + let tmpl = new Template( + "./test/stubs/classfields-data.11ty.js", + "./test/stubs/", + "./dist" + ); + + t.is(await tmpl.getOutputPath(), "./dist/classfields-data/index.html"); + let data = await tmpl.getData(); + + let pages = await tmpl.getRenderedTemplates(data); + t.is(pages[0].templateContent.trim(), "

Ted

"); + }); +} + test("JavaScript template type (class with shorthand data method)", async t => { let tmpl = new Template( "./test/stubs/class-data-fn-shorthand.11ty.js", @@ -189,3 +206,31 @@ test("JavaScript template type (class with renderData)", async t => { "

StringTesthowdy Zach, meet Thanos

" ); }); + +test("JavaScript template type (should use the same class instance for data and render)", async t => { + let tmpl = new Template( + "./test/stubs/oneinstance.11ty.js", + "./test/stubs/", + "./dist" + ); + + let data = await tmpl.getData(); + let pages = await tmpl.getRenderedTemplates(data); + // the template renders the random number created in the class constructor + // the data returns the random number created in the class constructor + // if they are different, the class is not reused. + t.is(pages[0].templateContent.trim(), `

Ted${data.rand}

`); +}); + +// TODO needs way more tests +test("JavaScript template type (multiple exports)", async t => { + let tmpl = new Template( + "./test/stubs/multipleexports.11ty.js", + "./test/stubs/", + "./dist" + ); + + let data = await tmpl.getData(); + let pages = await tmpl.getRenderedTemplates(data); + t.is(pages[0].templateContent.trim(), "

Ted

"); +}); diff --git a/test/stubs/classfields-data.11ty.js b/test/stubs/classfields-data.11ty.js new file mode 100644 index 000000000..78f93f73c --- /dev/null +++ b/test/stubs/classfields-data.11ty.js @@ -0,0 +1,11 @@ +class Test { + data = { + name: "Ted" + }; + + render({ name }) { + return `

${name}

`; + } +} + +module.exports = Test; diff --git a/test/stubs/multipleexports.11ty.js b/test/stubs/multipleexports.11ty.js new file mode 100644 index 000000000..28be6e7a8 --- /dev/null +++ b/test/stubs/multipleexports.11ty.js @@ -0,0 +1,7 @@ +exports.data = { + name: "Ted" +}; + +exports.render = function({ name }) { + return `

${name}

`; +}; diff --git a/test/stubs/oneinstance.11ty.js b/test/stubs/oneinstance.11ty.js new file mode 100644 index 000000000..19c403d38 --- /dev/null +++ b/test/stubs/oneinstance.11ty.js @@ -0,0 +1,18 @@ +class Test { + constructor() { + this.rand = Math.random(); + } + + get data() { + return { + name: "Ted", + rand: this.rand + }; + } + + render({ name }) { + return `

${name}${this.rand}

`; + } +} + +module.exports = Test; From 752b39ace127ab0a258785ca885621aaa6fada3d Mon Sep 17 00:00:00 2001 From: Zach Leatherman Date: Thu, 25 Jul 2019 08:19:21 -0500 Subject: [PATCH 2/4] Adds simplification code suggested by this awesome feedback from @jakearchibald https://github.com/11ty/eleventy/issues/622#issuecomment-514955817 --- src/Engines/JavaScript.js | 82 +++++++++++++++------------------------ 1 file changed, 32 insertions(+), 50 deletions(-) diff --git a/src/Engines/JavaScript.js b/src/Engines/JavaScript.js index de3c0799d..6a44c4a74 100644 --- a/src/Engines/JavaScript.js +++ b/src/Engines/JavaScript.js @@ -15,22 +15,33 @@ class JavaScript extends TemplateEngine { return result; } + _getInstance(mod) { + if (typeof mod === "string" || mod instanceof Buffer || mod.then) { + return { render: () => mod }; + } else if (typeof mod === "function") { + if ( + mod.prototype && + ("data" in mod.prototype || "render" in mod.prototype) + ) { + return new mod(); + } else { + return { render: mod }; + } + } else if ("data" in mod || "render" in mod) { + return mod; + } + } + getInstanceFromInputPath(inputPath) { if (this.instances[inputPath]) { return this.instances[inputPath]; } - const cls = this._getRequire(inputPath); - if (typeof cls === "function") { - if ( - cls.prototype && - ("data" in cls.prototype || "render" in cls.prototype) - ) { - let inst = new cls(); - this.instances[inputPath] = inst; - return inst; - } - } + const mod = this._getRequire(inputPath); + let inst = this._getInstance(mod); + + this.instances[inputPath] = inst; + return inst; } _getRequire(inputPath) { @@ -56,57 +67,28 @@ class JavaScript extends TemplateEngine { async getExtraDataFromFile(inputPath) { let inst = this.getInstanceFromInputPath(inputPath); - if (inst) { + if (inst && "data" in inst) { // get extra data from `data` method, // either as a function or getter or object literal return typeof inst.data === "function" ? await inst.data() : inst.data; } - - const cls = this._getRequire(inputPath); - if (typeof cls === "object") { - return typeof cls.data === "function" ? await cls.data() : cls.data; - } } async compile(str, inputPath) { - // for permalinks + let inst; if (str) { - // works with String, Buffer, Function! - return function(data) { - let target = str; - if (typeof str === "function") { - target = str.call(this.config.javascriptFunctions, data); - } - return this.normalize(target); - }.bind(this); + // When str has a value, it's being used for permalinks in data + inst = this._getInstance(str); + } else { + // For normal templates, str will be falsy. + inst = this.getInstanceFromInputPath(inputPath); } - // for all other requires, str will be falsy - const cls = this._getRequire(inputPath); - if (typeof cls === "function") { - // class with a `render` method - if (cls.prototype && "render" in cls.prototype) { - let inst = this.getInstanceFromInputPath(inputPath); - Object.assign(inst, this.config.javascriptFunctions); - return function(data) { - return this.normalize(inst.render.call(inst, data)); - }.bind(this); - } + if (inst && "render" in inst) { + Object.assign(inst, this.config.javascriptFunctions); - // raw function - return function(data) { - return this.normalize(cls.call(this.config.javascriptFunctions, data)); - }.bind(this); - } else if (typeof cls === "object" && "render" in cls) { return function(data) { - return this.normalize( - cls.render.call(this.config.javascriptFunctions, data) - ); - }.bind(this); - } else { - // string type does not work with javascriptFunctions - return function() { - return this.normalize(cls); + return this.normalize(inst.render.call(inst, data)); }.bind(this); } } From 60bc4bbdf2437e0330bffbe73f7efc974a7ac4aa Mon Sep 17 00:00:00 2001 From: Zach Leatherman Date: Thu, 25 Jul 2019 10:08:49 -0500 Subject: [PATCH 3/4] Fixes https://github.com/11ty/eleventy/commit/752b39ace127ab0a258785ca885621aaa6fada3d#r34445791 --- src/Engines/JavaScript.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Engines/JavaScript.js b/src/Engines/JavaScript.js index 6a44c4a74..624292b72 100644 --- a/src/Engines/JavaScript.js +++ b/src/Engines/JavaScript.js @@ -70,7 +70,10 @@ class JavaScript extends TemplateEngine { if (inst && "data" in inst) { // get extra data from `data` method, // either as a function or getter or object literal - return typeof inst.data === "function" ? await inst.data() : inst.data; + let result = await (typeof inst.data === "function" + ? inst.data() + : inst.data); + return result; } } From 598397ec7608fb48c87619e1a92f783bb1cac399 Mon Sep 17 00:00:00 2001 From: Zach Leatherman Date: Thu, 25 Jul 2019 17:32:08 -0500 Subject: [PATCH 4/4] More tests for #622. Proceed gracefully if a render method is missing from class or object definition --- src/Engines/JavaScript.js | 13 +++++ test/TemplateTest-JavaScript.js | 57 ++++++++++++++++++++- test/stubs/class-norender.11ty.js | 9 ++++ test/stubs/multipleexports-promises.11ty.js | 15 ++++++ test/stubs/object-norender.11ty.js | 5 ++ test/stubs/object.11ty.js | 8 +++ 6 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 test/stubs/class-norender.11ty.js create mode 100644 test/stubs/multipleexports-promises.11ty.js create mode 100644 test/stubs/object-norender.11ty.js create mode 100644 test/stubs/object.11ty.js diff --git a/src/Engines/JavaScript.js b/src/Engines/JavaScript.js index 624292b72..272b2743c 100644 --- a/src/Engines/JavaScript.js +++ b/src/Engines/JavaScript.js @@ -15,7 +15,14 @@ class JavaScript extends TemplateEngine { return result; } + // String, Buffer, Promise + // Function, Class + // Object _getInstance(mod) { + let noop = function() { + return ""; + }; + if (typeof mod === "string" || mod instanceof Buffer || mod.then) { return { render: () => mod }; } else if (typeof mod === "function") { @@ -23,11 +30,17 @@ class JavaScript extends TemplateEngine { mod.prototype && ("data" in mod.prototype || "render" in mod.prototype) ) { + if (!("render" in mod.prototype)) { + mod.prototype.render = noop; + } return new mod(); } else { return { render: mod }; } } else if ("data" in mod || "render" in mod) { + if (!("render" in mod)) { + mod.render = noop; + } return mod; } } diff --git a/test/TemplateTest-JavaScript.js b/test/TemplateTest-JavaScript.js index 56e8b5347..86de83f90 100644 --- a/test/TemplateTest-JavaScript.js +++ b/test/TemplateTest-JavaScript.js @@ -222,7 +222,6 @@ test("JavaScript template type (should use the same class instance for data and t.is(pages[0].templateContent.trim(), `

Ted${data.rand}

`); }); -// TODO needs way more tests test("JavaScript template type (multiple exports)", async t => { let tmpl = new Template( "./test/stubs/multipleexports.11ty.js", @@ -234,3 +233,59 @@ test("JavaScript template type (multiple exports)", async t => { let pages = await tmpl.getRenderedTemplates(data); t.is(pages[0].templateContent.trim(), "

Ted

"); }); + +test("JavaScript template type (multiple exports, promises)", async t => { + let tmpl = new Template( + "./test/stubs/multipleexports-promises.11ty.js", + "./test/stubs/", + "./dist" + ); + + let data = await tmpl.getData(); + t.is(data.name, "Ted"); + + let pages = await tmpl.getRenderedTemplates(data); + t.is(pages[0].templateContent.trim(), "

Ted

"); +}); + +test("JavaScript template type (object)", async t => { + let tmpl = new Template( + "./test/stubs/object.11ty.js", + "./test/stubs/", + "./dist" + ); + + let data = await tmpl.getData(); + t.is(data.name, "Ted"); + + let pages = await tmpl.getRenderedTemplates(data); + t.is(pages[0].templateContent.trim(), "

Ted

"); +}); + +test("JavaScript template type (object, no render method)", async t => { + let tmpl = new Template( + "./test/stubs/object-norender.11ty.js", + "./test/stubs/", + "./dist" + ); + + let data = await tmpl.getData(); + t.is(data.name, "Ted"); + + let pages = await tmpl.getRenderedTemplates(data); + t.is(pages[0].templateContent.trim(), ""); +}); + +test("JavaScript template type (class, no render method)", async t => { + let tmpl = new Template( + "./test/stubs/class-norender.11ty.js", + "./test/stubs/", + "./dist" + ); + + let data = await tmpl.getData(); + t.is(data.name, "Ted"); + + let pages = await tmpl.getRenderedTemplates(data); + t.is(pages[0].templateContent.trim(), ""); +}); diff --git a/test/stubs/class-norender.11ty.js b/test/stubs/class-norender.11ty.js new file mode 100644 index 000000000..a6962bb44 --- /dev/null +++ b/test/stubs/class-norender.11ty.js @@ -0,0 +1,9 @@ +class Test { + data() { + return { + name: "Ted" + }; + } +} + +module.exports = Test; diff --git a/test/stubs/multipleexports-promises.11ty.js b/test/stubs/multipleexports-promises.11ty.js new file mode 100644 index 000000000..995f7f4ad --- /dev/null +++ b/test/stubs/multipleexports-promises.11ty.js @@ -0,0 +1,15 @@ +exports.data = async function() { + return new Promise((resolve, reject) => { + setTimeout(function() { + resolve({ name: "Ted" }); + }, 100); + }); +}; + +exports.render = async function({ name }) { + return new Promise((resolve, reject) => { + setTimeout(function() { + resolve(`

${name}

`); + }, 100); + }); +}; diff --git a/test/stubs/object-norender.11ty.js b/test/stubs/object-norender.11ty.js new file mode 100644 index 000000000..b9a29f697 --- /dev/null +++ b/test/stubs/object-norender.11ty.js @@ -0,0 +1,5 @@ +module.exports = { + data: { + name: "Ted" + } +}; diff --git a/test/stubs/object.11ty.js b/test/stubs/object.11ty.js new file mode 100644 index 000000000..75ff39494 --- /dev/null +++ b/test/stubs/object.11ty.js @@ -0,0 +1,8 @@ +module.exports = { + data: { + name: "Ted" + }, + render: function({ name }) { + return `

${name}

`; + } +};