From 5651392462ee0ff19d77c8481081a99e5b9138dd Mon Sep 17 00:00:00 2001 From: Ben Gubler Date: Fri, 27 Jan 2023 20:29:45 -0700 Subject: [PATCH] Don't use data object for Eta configuration (#214) --- deno_dist/file-handlers.ts | 42 +++++++------------------------------- src/file-handlers.ts | 42 +++++++------------------------------- test/file-handlers.spec.ts | 26 ++++------------------- 3 files changed, 18 insertions(+), 92 deletions(-) diff --git a/deno_dist/file-handlers.ts b/deno_dist/file-handlers.ts index 575d21e..168acea 100644 --- a/deno_dist/file-handlers.ts +++ b/deno_dist/file-handlers.ts @@ -4,7 +4,6 @@ import EtaErr from "./err.ts"; import compile from "./compile.ts"; import { getConfig } from "./config.ts"; import { getPath, readFile } from "./file-utils.ts"; -import { copyProps } from "./utils.ts"; import { promiseImpl } from "./polyfills.ts"; /* TYPES */ @@ -19,10 +18,6 @@ import type { TemplateFunction } from "./compile.ts"; export type CallbackFn = (err: Error | null, str?: string) => void; interface DataObj { - /** Express.js settings may be stored here */ - settings?: { - [key: string]: any; // eslint-disable-line @typescript-eslint/no-explicit-any - }; [key: string]: any; // eslint-disable-line @typescript-eslint/no-explicit-any } @@ -173,8 +168,7 @@ function includeFile( * * This can take two different function signatures: * - * - `renderFile(filename, dataAndConfig, [cb])` - * - Eta will merge `dataAndConfig` into `eta.config` + * - `renderFile(filename, data, [cb])` * - `renderFile(filename, data, [config], [cb])` * * Note that renderFile does not immediately return the rendered result. If you pass in a callback function, it will be called with `(err, res)`. Otherwise, `renderFile` will return a `Promise` that resolves to the render result. @@ -189,7 +183,6 @@ function includeFile( * * let rendered = await eta.renderFile("./template.eta", data, {cache: true}) * - * let rendered = await eta.renderFile("./template", {...data, cache: true}) * ``` */ @@ -224,14 +217,13 @@ function renderFile( /* Here we have some function overloading. Essentially, the first 2 arguments to renderFile should always be the filename and data - However, with Express, configuration options will be passed along with the data. - Thus, Express will call renderFile with (filename, dataAndOptions, cb) - And we want to also make (filename, data, options, cb) available + Express will call renderFile with (filename, data, cb) + We also want to make (filename, data, options, cb) available */ let renderConfig: EtaConfigWithFilename; let callback: CallbackFn | undefined; - data = data || {}; // If data is undefined, we don't want accessing data.settings to error + data = data || {}; // First, assign our callback function to `callback` // We can leave it undefined if neither parameter is a function; @@ -250,26 +242,8 @@ function renderFile( (config as PartialConfig) || {}, ) as EtaConfigWithFilename; } else { - // Otherwise, get the config from the data object - // And then grab some config options from data.settings - // Which is where Express sometimes stores them - renderConfig = getConfig(data as PartialConfig) as EtaConfigWithFilename; - if (data.settings) { - // Pull a few things from known locations - if (data.settings.views) { - renderConfig.views = data.settings.views; - } - if (data.settings["view cache"]) { - renderConfig.cache = true; - } - // Undocumented after Express 2, but still usable, esp. for - // items that are unsafe to be passed along with data, like `root` - const viewOpts = data.settings["view options"]; - - if (viewOpts) { - copyProps(renderConfig, viewOpts); - } - } + // Otherwise, get the default config + renderConfig = getConfig({}) as EtaConfigWithFilename; } // Set the filename option on the template @@ -286,8 +260,7 @@ function renderFile( * * This can take two different function signatures: * - * - `renderFile(filename, dataAndConfig, [cb])` - * - Eta will merge `dataAndConfig` into `eta.config` + * - `renderFile(filename, data, [cb])` * - `renderFile(filename, data, [config], [cb])` * * Note that renderFile does not immediately return the rendered result. If you pass in a callback function, it will be called with `(err, res)`. Otherwise, `renderFile` will return a `Promise` that resolves to the render result. @@ -302,7 +275,6 @@ function renderFile( * * let rendered = await eta.renderFile("./template.eta", data, {cache: true}) * - * let rendered = await eta.renderFile("./template", {...data, cache: true}) * ``` */ diff --git a/src/file-handlers.ts b/src/file-handlers.ts index 3a1c1b3..39534a6 100644 --- a/src/file-handlers.ts +++ b/src/file-handlers.ts @@ -4,7 +4,6 @@ import EtaErr from "./err.js"; import compile from "./compile.js"; import { getConfig } from "./config.js"; import { getPath, readFile } from "./file-utils.js"; -import { copyProps } from "./utils.js"; import { promiseImpl } from "./polyfills.js"; /* TYPES */ @@ -15,10 +14,6 @@ import type { TemplateFunction } from "./compile.js"; export type CallbackFn = (err: Error | null, str?: string) => void; interface DataObj { - /** Express.js settings may be stored here */ - settings?: { - [key: string]: any; // eslint-disable-line @typescript-eslint/no-explicit-any - }; [key: string]: any; // eslint-disable-line @typescript-eslint/no-explicit-any } @@ -150,8 +145,7 @@ function includeFile(path: string, options: EtaConfig): [TemplateFunction, EtaCo * * This can take two different function signatures: * - * - `renderFile(filename, dataAndConfig, [cb])` - * - Eta will merge `dataAndConfig` into `eta.config` + * - `renderFile(filename, data, [cb])` * - `renderFile(filename, data, [config], [cb])` * * Note that renderFile does not immediately return the rendered result. If you pass in a callback function, it will be called with `(err, res)`. Otherwise, `renderFile` will return a `Promise` that resolves to the render result. @@ -166,7 +160,6 @@ function includeFile(path: string, options: EtaConfig): [TemplateFunction, EtaCo * * let rendered = await eta.renderFile("./template.eta", data, {cache: true}) * - * let rendered = await eta.renderFile("./template", {...data, cache: true}) * ``` */ @@ -192,14 +185,13 @@ function renderFile( /* Here we have some function overloading. Essentially, the first 2 arguments to renderFile should always be the filename and data - However, with Express, configuration options will be passed along with the data. - Thus, Express will call renderFile with (filename, dataAndOptions, cb) - And we want to also make (filename, data, options, cb) available + Express will call renderFile with (filename, data, cb) + We also want to make (filename, data, options, cb) available */ let renderConfig: EtaConfigWithFilename; let callback: CallbackFn | undefined; - data = data || {}; // If data is undefined, we don't want accessing data.settings to error + data = data || {}; // First, assign our callback function to `callback` // We can leave it undefined if neither parameter is a function; @@ -216,26 +208,8 @@ function renderFile( if (typeof config === "object") { renderConfig = getConfig((config as PartialConfig) || {}) as EtaConfigWithFilename; } else { - // Otherwise, get the config from the data object - // And then grab some config options from data.settings - // Which is where Express sometimes stores them - renderConfig = getConfig(data as PartialConfig) as EtaConfigWithFilename; - if (data.settings) { - // Pull a few things from known locations - if (data.settings.views) { - renderConfig.views = data.settings.views; - } - if (data.settings["view cache"]) { - renderConfig.cache = true; - } - // Undocumented after Express 2, but still usable, esp. for - // items that are unsafe to be passed along with data, like `root` - const viewOpts = data.settings["view options"]; - - if (viewOpts) { - copyProps(renderConfig, viewOpts); - } - } + // Otherwise, get the default config + renderConfig = getConfig({}) as EtaConfigWithFilename; } // Set the filename option on the template @@ -252,8 +226,7 @@ function renderFile( * * This can take two different function signatures: * - * - `renderFile(filename, dataAndConfig, [cb])` - * - Eta will merge `dataAndConfig` into `eta.config` + * - `renderFile(filename, data, [cb])` * - `renderFile(filename, data, [config], [cb])` * * Note that renderFile does not immediately return the rendered result. If you pass in a callback function, it will be called with `(err, res)`. Otherwise, `renderFile` will return a `Promise` that resolves to the render result. @@ -268,7 +241,6 @@ function renderFile( * * let rendered = await eta.renderFile("./template.eta", data, {cache: true}) * - * let rendered = await eta.renderFile("./template", {...data, cache: true}) * ``` */ diff --git a/test/file-handlers.spec.ts b/test/file-handlers.spec.ts index 560d339..9aa68d9 100644 --- a/test/file-handlers.spec.ts +++ b/test/file-handlers.spec.ts @@ -67,28 +67,10 @@ describe("Simple renderFile tests", () => { templates.define(fakeFilePath, compile("This template does not exist")); // renderFile should just look straight in the cache for the template - renderFile(fakeFilePath, { cache: true }, function (_err: Error | null, res?: string) { + renderFile(fakeFilePath, {}, { cache: true }, function (_err: Error | null, res?: string) { expect(res).toEqual("This template does not exist"); }); }); - - it("parses a simple template w/ settings from Express", async () => { - renderFile( - filePath, - { - name: "

Ben

", - cache: true, - settings: { - views: [path.join(__dirname, "templates"), path.join(__dirname, "othertemplates")], - "view cache": true, - "view options": { autoEscape: false }, - }, - }, - function (_err: Error | null, res?: string) { - expect(res).toEqual("Hi

Ben

"); - } - ); - }); }); describe("File location tests", () => { @@ -105,9 +87,9 @@ describe("File location tests", () => { describe("renderFile error tests", () => { it("render file with callback works on error", (done) => { - function cb(err: Error, _res?: string) { + function cb(err: Error | null, _res?: string) { expect(err).toBeTruthy(); - expect(err.message).toMatch( + expect(err?.message).toMatch( buildRegEx(` var tR='',__l,__lP,include=E.include.bind(E),includeFile=E.includeFile.bind(E) function layout(p,d){__l=p;__lP=d} @@ -120,7 +102,7 @@ if(cb){cb(null,tR)} return tR done(); } - renderFile(errFilePath, { name: "Ada Lovelace", async: true }, cb); + renderFile(errFilePath, { name: "Ada Lovelace" }, { async: true }, cb); }); test("throws with bad inner JS syntax using Promises", async () => {