Merge pull request #283 from cdloh/fix/customesi-doesnt-have-to-be-a-promise

fix: Cleanup useless async and awaits & custom ESI Vars function doesn't have to be async

Author: cdloh

README.md

@@ -151,22 +151,22 @@ This is useful if your client is sending a sensitive cookie that you don't ever
 ### Custom ESI Vars Function
 
 ```
-export type customESIVarsFunction = (request: Request) => Promise<customESIVars>;
+export type customESIVarsFunction = (request: Request) => Promise<customESIVars> | customESIVars;
 export type customESIVars = {
    [key: string]: string | { [key: string]: string };
 };
 ```
 
 If you want to inject custom ESI vars into the parser per request you can pass the class a custom async function that will be evaluated each request.
 
-The async function accepts a request object and returns an object.
+The function accepts a request object, returns an object and can be async.
 
 The object values can either be strings or objects. If the value is an object it the ESIVar must be refrenced with a key in the ESI variable or the default variable will be returned.
 
 eg that pulls GEOIP data out of the Request and injects it as `GEOIP_X` ESI Vars
 
 ```javascript
-const geoIPVarsFromRequest = async function (request) {
+const geoIPVarsFromRequest = function (request) {
   let customVariables = {};
   let cfData = request.cf;
   let geoipVars = [

package.json

@@ -56,6 +56,9 @@
   "license": "BSD-2-Clause",
   "eslintConfig": {
     "root": true,
+    "parserOptions": {
+      "project": "./tsconfig.json"
+    },
     "extends": [
       "typescript",
       "prettier",
@@ -73,15 +76,11 @@
           "startLines": 1
         }
       ],
-      "no-new-func": [
-        "error"
-      ],
-      "no-eval": [
-        "error"
-      ],
-      "no-return-await": [
-        "error"
-      ]
+      "no-new-func": "error",
+      "no-eval": "error",
+      "no-return-await": "error",
+      "@typescript-eslint/await-thenable": "error",
+      "require-await": "error"
     }
   },
   "devDependencies": {

src/handleChunk.ts

@@ -8,7 +8,7 @@ import { tagParser } from "./tagParser";
  * @returns {void}
  */
 type writerFunction = (text: string, esi: boolean) => void;
-type handleFunction = (value: string, done: boolean) => Promise<void>;
+type handleFunction = (value: string, done: boolean) => void;
 
 /**
  *  Creates a chunk handler and returns a
@@ -26,13 +26,13 @@ export function create(writer: writerFunction): handleFunction {
     }
   };
 
-  return async function (value: string, done: boolean): Promise<void> {
+  return function (value: string, done: boolean): void {
     value = prev_chunk + value;
     prev_chunk = "";
 
     const parser = new tagParser(value);
     do {
-      const [tag, before, after] = await parser.next();
+      const [tag, before, after] = parser.next();
 
       // Always write before if we have it
       writeString(before);
@@ -64,8 +64,8 @@ export function create(writer: writerFunction): handleFunction {
       // eslint-disable-next-line no-constant-condition
     } while (true);
 
-    // Check if we had something left over
-    // But we didnt write it
+    // ensure we write anything left over
+    // when we are done
     if (done) {
       writeString(prev_chunk);
     }

src/index.ts

@@ -21,10 +21,10 @@ import { getheaderToken } from "./headerUtils";
  * @property {string[]} contentTypes - Array of strings of content types that the parser should parse for ESI Tags
  * Note: That these are case sensitive. See - https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Type
  * @property {boolean} disableThirdPartyIncludes - Whether or not to enable third party includes (includes from other domains)
- * @property {number} [recursionLimit] - Levels of recusion the parser is allowed to go do
- * think includes that include themselves causing recusion
+ * @property {number} [recursionLimit] - Levels of recursion the parser is allowed to include before bailing out
+ * think includes that include themselves causing recursion
  * @default 10
- * @property {string[]} thirdPatyIncludesDomainWhitelist - If third party includes are disabled you can white list them by including domains here
+ * @property {string[]} thirdPartyIncludesDomainWhitelist - If third party includes are disabled you can white list them by including domains here
  * @property {string[]} varsCookieBlacklist - Array of strings of cookies that will be blacklisted from being expanded in esi VARs.
  */
 export type ESIConfig = {
@@ -47,14 +47,14 @@ export type ESIVars = {
  * ESI Event Data for the Current Request
  *
  * @property {ESIConfig} config - ESIConfig when class was created
- * @property {object} headers - All headers of the request uppercased
+ * @property {object} headers - All headers of the request in uppercase
  * @property {string} method - Method of the request
  * @property {URLSearchParams} esiArgs - Any ESI Arguments extracted from the URL Search params of the original request
  * Will be a URLSearchParam encoded object
  * @property {customESIVars} customVars - If a custom ESI Vars function is supplied this will be the result of that function
  * @property {URL} url - URL Object of the Request with ESI Args removed
  * @property {Request} request - Request object after ESI Args have been removed
- * @property {number} recursion - Recusion level we're currently at
+ * @property {number} recursion - Recursion level we're currently at
  */
 export type ESIEventData = {
   /**
@@ -63,7 +63,7 @@ export type ESIEventData = {
   config: ESIConfig;
   /**
    * All headers of the current Request in {Object}
-   * All headers are uppercassed with - being converted to _
+   * All headers are in uppercase and - is converted to _
    */
   headers: { [key: string]: string };
   /**
@@ -101,7 +101,7 @@ export type customESIVars = {
 };
 export type customESIVarsFunction = (
   request: Request,
-) => Promise<customESIVars>;
+) => Promise<customESIVars> | customESIVars;
 export type fetchFunction = (
   request: RequestInfo,
   ctx: Request[],
@@ -150,7 +150,7 @@ export class esi {
     // Remove ESI Vars if they're in the request
     // Return a brand new request
     // eslint-disable-next-line
-    let [request, esiVars] = await getVars(origRequest);
+    let [request, esiVars] = getVars(origRequest);
 
     // Load custom values if we can
     let customESIVariables;
@@ -159,7 +159,7 @@ export class esi {
     }
 
     // Add SurrogateControl header or append to it
-    request = await advertiseSurrogateControl(request);
+    request = advertiseSurrogateControl(request);
 
     // pack our nice stuff in
     const eventData: ESIEventData = {
@@ -189,7 +189,7 @@ export class esi {
       !response.body ||
       !this.validContentType(response) ||
       !this.validSurrogateControl(response) ||
-      (await canDelegateToSurrogate(origRequest, this.options))
+      canDelegateToSurrogate(origRequest, this.options)
     ) {
       const resp = new Response(response.body, response);
       // We set the URL manually here as it doesn't come across from the copy˛
@@ -230,10 +230,10 @@ export class esi {
   }
 
   async handleESI(eventData: ESIEventData, text: string): Promise<string> {
-    text = await processEscaping(text);
+    text = processEscaping(text);
     text = processComments(text);
     text = processRemove(text);
-    text = await processESIVars(eventData, text);
+    text = processESIVars(eventData, text);
     let vars = false;
     [text, vars] = await processConditionals(eventData, text);
 
@@ -249,15 +249,16 @@ export class esi {
 
     return text;
   }
+  // eslint-disable-next-line require-await
   async handleTEXT(eventData: ESIEventData, text: string): Promise<string> {
     return text;
   }
 
-  async streamBody(
+  streamBody(
     eventData: ESIEventData,
     readable: ReadableStream,
     writable: WritableStream,
-  ): Promise<void> {
+  ) {
     const reader = readable.getReader();
     const encoder = new TextEncoder();
     const decoder = new TextDecoder();
@@ -328,12 +329,12 @@ export class esi {
 
     const handler = createHandleChunk(writerBound);
 
-    reader.read().then(async function processBlob(blob): Promise<void> {
+    reader.read().then(function processBlob(blob): Promise<void> | undefined {
       const chunk: ArrayBuffer = blob.value;
       const done: boolean = blob.done;
       // decode it
       const decodedChunk: string = decoder.decode(chunk, { stream: true });
-      await handler(decodedChunk, done);
+      handler(decodedChunk, done);
 
       // we're done bail out
       if (done) {
@@ -390,9 +391,9 @@ const esiArgsRegex = /^esi_(\S+)/;
  * Return a brand new mutatable Request along with an ESIVars object
  *
  * @param {Request} request - Original Request
- * @returns {Promise<[Request, ESIVars]>} - Mutatable Request and ESIVars
+ * @returns {[Request, ESIVars]} - Mutatable Request and ESIVars
  */
-async function getVars(request: Request): Promise<[Request, ESIVars]> {
+function getVars(request: Request): [Request, ESIVars] {
   const vars: ESIVars = {
     headers: {},
     method: request.method,
@@ -466,7 +467,7 @@ export function getProcessorVersionString(): string {
  * @param {Request[]} ctx request ctx (parent requests)
  * @returns {Promise<Response>} - processor supported version
  */
-async function defaultFetch(
+function defaultFetch(
   req: RequestInfo,
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   ctx: Request[],

src/processConditionals.ts

@@ -23,7 +23,7 @@ export async function process(
   let hasConditionals = false;
 
   do {
-    const [choose, ch_before, ch_after] = await parser.next("esi:choose");
+    const [choose, ch_before, ch_after] = parser.next("esi:choose");
 
     if (choose && choose.closing) {
       hasConditionals = true;
@@ -43,7 +43,7 @@ export async function process(
       let otherwise = null;
 
       do {
-        const [tag, ,] = await innerParser.next("esi:when|esi:otherwise");
+        const [tag, ,] = innerParser.next("esi:when|esi:otherwise");
 
         if (tag && tag.closing && tag.whole && tag.contents) {
           if (tag.tagname == "esi:when" && !whenMatched) {

src/processESIVars.ts

@@ -13,12 +13,9 @@ const esiLessArrow = />/g;
  *
  * @param {ESIEventData} eventData event data for the current request
  * @param {string} chunk string to process
- * @returns {Promise<string>} processed string
+ * @returns {string} processed string
  */
-export async function process(
-  eventData: ESIEventData,
-  chunk: string,
-): Promise<string> {
+export function process(eventData: ESIEventData, chunk: string): string {
   if (chunk.indexOf("esi:vars") == -1) {
     return chunk;
   }

src/processEscaping.ts

@@ -5,17 +5,14 @@ import { tagParser } from "./tagParser";
  *
  * @param {string} chunk Chunk of text to process
  * @param {string[]} [res] Array of chunks of text already processed (used internally within function)
- * @returns {Promise<string>} processed string
+ * @returns {string} processed string
  */
-export async function process(
-  chunk: string,
-  res: Array<string> = [],
-): Promise<string> {
+export function process(chunk: string, res: Array<string> = []): string {
   const parser = new tagParser(chunk);
   let hasEscaping = false;
 
   do {
-    const [tag, before, after] = await parser.next("!--esi");
+    const [tag, before, after] = parser.next("!--esi");
 
     if (tag && tag.closing && tag.contents) {
       hasEscaping = true;

src/surrogate.ts

@@ -12,11 +12,9 @@ import { getheaderToken } from "./headerUtils";
  * If we have Colo data in the Request object we add that to our identifier
  *
  * @param {Request} request Request to modify
- * @returns {Promise<Request>} Request with SurrogateControl header added
+ * @returns {Request} Request with SurrogateControl header added
  */
-export async function advertiseSurrogateControl(
-  request: Request,
-): Promise<Request> {
+export function advertiseSurrogateControl(request: Request): Request {
   let coloName = "";
   if (request.cf && request.cf.colo) {
     coloName = `-${request.cf.colo}`;
@@ -33,12 +31,12 @@ export async function advertiseSurrogateControl(
  *
  * @param {Request} request Request to confirm against
  * @param {ESIConfig} config Config for the current request
- * @returns {Promise<boolean>} result
+ * @returns {boolean} result
  */
-export async function canDelegateToSurrogate(
+export function canDelegateToSurrogate(
   request: Request,
   config: ESIConfig,
-): Promise<boolean> {
+): boolean {
   const surrogates = config.allowSurrogateDelegation;
   if (surrogates === undefined || surrogates === false) return false;
 

src/tagParser.ts

@@ -23,9 +23,7 @@ export class tagParser {
     this.next = this.next.bind(this);
   }
 
-  async next(
-    tagname?: string,
-  ): Promise<[tag | null, string | undefined, string | undefined]> {
+  next(tagname?: string): [tag | null, string | undefined, string | undefined] {
     const tag = this.findWholeTag(tagname);
     let before, after;
 

test/esi.spec.ts

@@ -416,7 +416,7 @@ test("TEST 7h: Compile instructions if ESI delegation is enabled by IP but no Ca
   expect(await res.text()).toEqual(`a=1`);
 });
 
-test("TEST 8: Response downstrean cacheability is zeroed when ESI processing", async () => {
+test("TEST 8: Response downstream cacheability is zeroed when ESI processing", async () => {
   const url = `/esi/test-8`;
   routeHandler.add(url, function (req, res) {
     res.writeHead(200, {
@@ -1862,7 +1862,7 @@ test("TEST 48: POST With body", async () => {
   expect(await res.text()).toEqual(postBody);
 });
 
-test("TEST 48: Mutliple ESI Includes next to each other", async () => {
+test("TEST 48: Multiple ESI Includes next to each other", async () => {
   const url = `/esi/test-48`;
   routeHandler.add(url, function (req, res) {
     res.writeHead(200, esiHead);
@@ -1885,7 +1885,7 @@ test("TEST 48: Mutliple ESI Includes next to each other", async () => {
   expect(await res.text()).toEqual(`START:\nFRAGMENT\nFRAGMENT\n:END`);
 });
 
-test("TEST 49: Mutliple ESI Includes next to each other in choose", async () => {
+test("TEST 49: Multiple ESI Includes next to each other in choose", async () => {
   const url = `/esi/test-49`;
   routeHandler.add(url, function (req, res) {
     res.writeHead(200, esiHead);
@@ -1930,7 +1930,7 @@ describe("TEST 51: ESI Args that lead with ints shouldn't convert to ints", () =
     { arg: "1719,1732", result: "second-lineage" },
     { arg: "1719,1918", result: "third-lineage" },
     { arg: "1719,1922", result: "forth-lineage" },
-    { arg: "1719,1926", result: "fith-lineage" },
+    { arg: "1719,1926", result: "fifth-lineage" },
     { arg: "1719,2000", result: "sixth-lineage" },
     { arg: "2000", result: "seventh-lineage" },
     { arg: "2001", result: "eighth-lineage" },
@@ -1946,7 +1946,7 @@ describe("TEST 51: ESI Args that lead with ints shouldn't convert to ints", () =
     <esi:when test="$(ESI_ARGS{lineage}) =~ '/(?:^|,)(1732)(?:,|$)/'">second-lineage</esi:when>
     <esi:when test="$(ESI_ARGS{lineage}) =~ '/(?:^|,)(1918)(?:,|$)/'">third-lineage</esi:when>
     <esi:when test="$(ESI_ARGS{lineage}) =~ '/(?:^|,)(1922)(?:,|$)/'">forth-lineage</esi:when>
-    <esi:when test="$(ESI_ARGS{lineage}) =~ '/(?:^|,)(1926)(?:,|$)/'">fith-lineage</esi:when>
+    <esi:when test="$(ESI_ARGS{lineage}) =~ '/(?:^|,)(1926)(?:,|$)/'">fifth-lineage</esi:when>
     <esi:when test="$(ESI_ARGS{lineage}) == '1719,2000'">sixth-lineage</esi:when>
     <esi:when test="$(ESI_ARGS{lineage}) == '2000'">seventh-lineage</esi:when>
     <esi:when test="$(ESI_ARGS{lineage}) >= '800'">eighth-lineage</esi:when>