perf(gatsby): Enable fast filters for $regex/$glob (#24188)

Author: pvdz

packages/gatsby/src/redux/nodes.ts

@@ -4,17 +4,32 @@ import { createPageDependency } from "./actions/add-page-dependency"
 import { IDbQueryElemMatch } from "../db/common/query"
 
 // Only list supported ops here. "CacheableFilterOp"
-type FilterOp = "$eq" | "$ne" | "$lt" | "$lte" | "$gt" | "$gte" | "$in" | "$nin"
+type FilterOp =
+  | "$eq"
+  | "$ne"
+  | "$lt"
+  | "$lte"
+  | "$gt"
+  | "$gte"
+  | "$in"
+  | "$nin"
+  | "$regex" // Note: this includes $glob
 // Note: `undefined` is an encoding for a property that does not exist
 type FilterValueNullable =
   | string
   | number
   | boolean
   | null
   | undefined
+  | RegExp // Only valid for $regex
   | Array<string | number | boolean | null | undefined>
 // This is filter value in most cases
-type FilterValue = string | number | boolean | Array<string | number | boolean>
+type FilterValue =
+  | string
+  | number
+  | boolean
+  | RegExp // Only valid for $regex
+  | Array<string | number | boolean>
 export type FilterCacheKey = string
 export interface IFilterCache {
   op: FilterOp
@@ -697,6 +712,33 @@ export const getNodesFromCacheByValue = (
     return set
   }
 
+  if (op === `$regex`) {
+    // Note: $glob is converted to $regex so $glob filters go through here, too
+    // Aside from the input pattern format, further behavior is exactly the same.
+
+    // The input to the filter must be a string (including leading/trailing slash and regex flags)
+    // By the time the filter reaches this point, the filterValue has to be a regex.
+
+    if (!(filterValue instanceof RegExp)) {
+      throw new Error(
+        `The value for the $regex comparator must be an instance of RegExp`
+      )
+    }
+    const regex = filterValue
+
+    const result = new Set<IGatsbyNode>()
+    filterCache.byValue.forEach((nodes, value) => {
+      // TODO: does the value have to be a string for $regex? Can we auto-ignore any non-strings? Or does it coerce.
+      // Note: partial paths should also be included for regex (matching Sift behavior)
+      if (value !== undefined && regex.test(String(value))) {
+        nodes.forEach(node => result.add(node))
+      }
+    })
+
+    // TODO: we _can_ cache this set as well. Might make sense if it turns out that $regex is mostly used with literals
+    return result
+  }
+
   if (filterValue == null) {
     if (op === `$lt` || op === `$gt`) {
       // Nothing is lt/gt null
@@ -714,6 +756,14 @@ export const getNodesFromCacheByValue = (
     )
   }
 
+  if (filterValue instanceof RegExp) {
+    // This is most likely an internal error, although it is possible for
+    // users to talk to this API more directly.
+    throw new Error(
+      `A RegExp instance is only valid for $regex and $glob comparators`
+    )
+  }
+
   if (op === `$lt`) {
     // First try a direct approach. If a value is queried that also exists then
     // we can prevent a binary search through the whole set, O(1) vs O(log n)

packages/gatsby/src/redux/run-sift.js

@@ -19,7 +19,17 @@ const {
   getNode: siftGetNode,
 } = require(`./nodes`)
 
-const FAST_OPS = [`$eq`, `$ne`, `$lt`, `$lte`, `$gt`, `$gte`, `$in`, `$nin`]
+const FAST_OPS = [
+  `$eq`,
+  `$ne`,
+  `$lt`,
+  `$lte`,
+  `$gt`,
+  `$gte`,
+  `$in`,
+  `$nin`,
+  `$regex`, // Note: this includes $glob
+]
 
 // More of a testing mechanic, to verify whether last runSift call used Sift
 let lastFilterUsedSift = false

packages/gatsby/src/schema/__tests__/run-query.js

@@ -1119,7 +1119,7 @@ it(`should use the cache argument`, async () => {
         it(`handles the regex operator without flags`, async () => {
           const needleStr = `/^The.*Wax/`
           const needleRex = /^The.*Wax/
-          const [result, allNodes] = await runSlowFilter({
+          const [result, allNodes] = await runFastFilter({
             name: { regex: needleStr },
           })
 
@@ -1136,7 +1136,46 @@ it(`should use the cache argument`, async () => {
           // Note: needle is different from checked because `new RegExp('/a/i')` does _not_ work
           const needleRex = /^the.*wax/i
           const needleStr = `/^the.*wax/i`
-          const [result, allNodes] = await runSlowFilter({
+          const [result, allNodes] = await runFastFilter({
+            name: { regex: needleStr },
+          })
+
+          expect(result?.length).toEqual(
+            allNodes.filter(node => needleRex.test(node.name)).length
+          )
+          expect(result?.length).toBeGreaterThan(0) // Make sure there _are_ results, don't let this be zero
+          result.forEach(node =>
+            expect(needleRex.test(node.name)).toEqual(true)
+          )
+        })
+
+        it(`rejects strings without forward slashes`, async () => {
+          await expect(
+            runFastFilter({
+              name: { regex: `^The.*Wax` },
+            })
+          ).rejects.toThrow()
+        })
+
+        it(`rejects strings without trailing slash`, async () => {
+          await expect(
+            runFastFilter({
+              name: { regex: `/^The.*Wax` },
+            })
+          ).rejects.toThrow()
+        })
+
+        it(`accepts strings without leading slash`, async () => {
+          // If this test starts failing, that might be okay and it should be dropped.
+          // At the time of writing it, this was a status quo edge case for Sift
+
+          // Passes because the requirement is mostly about a .split() failing
+          // for the other fail cases. As long as it passes a regex ultimately
+          // we don't really have to care / validate.
+
+          const needleRex = /(?:)/i // This was what it turned into
+          const needleStr = `^the.*wax/i`
+          const [result, allNodes] = await runFastFilter({
             name: { regex: needleStr },
           })
 
@@ -1149,22 +1188,34 @@ it(`should use the cache argument`, async () => {
           )
         })
 
-        it(`handles the nested regex operator`, async () => {
+        it(`rejects actual regex`, async () => {
+          await expect(
+            runFastFilter({
+              name: { regex: /^The.*Wax/ },
+            })
+          ).rejects.toThrow()
+        })
+
+        it(`handles the nested regex operator and ignores partial paths`, async () => {
           const needleStr = `/.*/`
           const needleRex = /.*/
-          const [result, allNodes] = await runSlowFilter({
+          const [result, allNodes] = await runFastFilter({
             nestedRegex: { field: { regex: needleStr } },
           })
 
           expect(result?.length).toEqual(
             allNodes.filter(
-              node => node.nestedRegex && needleRex.test(node.nestedRegex.field)
+              node =>
+                node.nestedRegex !== undefined &&
+                node.nestedRegex.field !== undefined &&
+                needleRex.test(node.nestedRegex.field)
             ).length
           )
           expect(result?.length).toBeGreaterThan(0) // Make sure there _are_ results, don't let this be zero
           result.forEach(node =>
             expect(
-              node.nestedRegex && needleRex.test(node.nestedRegex.field)
+              node.nestedRegex === undefined ||
+                needleRex.test(node.nestedRegex.field)
             ).toEqual(true)
           )
         })
@@ -1726,8 +1777,10 @@ it(`should use the cache argument`, async () => {
       })
 
       describe(`$glob`, () => {
+        // Note: glob internally uses $regex
+
         it(`handles the glob operator`, async () => {
-          const [result] = await runSlowFilter({ name: { glob: `*Wax` } })
+          const [result] = await runFastFilter({ name: { glob: `*Wax` } })
 
           expect(result.length).toEqual(2)
           expect(result[0].name).toEqual(`The Mad Wax`)