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

Don't use data object for Eta configuration #214

Merged
merged 4 commits into from Jan 28, 2023
Merged
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
42 changes: 7 additions & 35 deletions deno_dist/file-handlers.ts
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
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
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