Skip to content

Commit

Permalink
Don't use data object for Eta configuration (#214)
Browse files Browse the repository at this point in the history
  • Loading branch information
@nebrelbug
nebrelbug authored Jan 28, 2023
1 parent 811b6be commit 5651392
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 92 deletions.
42 changes: 7 additions & 35 deletions deno_dist/file-handlers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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
}

Expand Down Expand Up @@ -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.
Expand All @@ -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})
* ```
*/

Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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})
* ```
*/

Expand Down
42 changes: 7 additions & 35 deletions src/file-handlers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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
}

Expand Down Expand Up @@ -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.
Expand All @@ -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})
* ```
*/

Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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})
* ```
*/

Expand Down
26 changes: 4 additions & 22 deletions test/file-handlers.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: "<p>Ben</p>",
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 <p>Ben</p>");
}
);
});
});

describe("File location tests", () => {
Expand All @@ -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}
Expand All @@ -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 () => {
Expand Down

0 comments on commit 5651392

Please sign in to comment.