Skip to content

Commit 6ee4a35

Browse files
pvdzpvdz
authored
perf(gatsby): stop using Sift as fallback (#24743)
1 parent 7e211f7 commit 6ee4a35

File tree

5 files changed

+45
-35
lines changed

5 files changed

+45
-35
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ describe(`run-sift tests`, () => {
132132
})
133133
;[
134134
{ desc: `with cache`, cb: () /*:FiltersCache*/ => new Map() }, // Avoids sift for flat filters
135-
{ desc: `no cache`, cb: () => null }, // Always goes through sift
135+
// { desc: `no cache`, cb: () => null }, // Always goes through sift
136136
].forEach(({ desc, cb: createFiltersCache }) => {
137137
describe(desc, () => {
138138
describe(`filters by just id correctly`, () => {

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

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -467,20 +467,28 @@ const applyFilters = (
467467
}
468468
lastFilterUsedSift = true
469469

470-
const siftResult /*: Array<IGatsbyNode> | null */ = filterWithSift(
471-
filters,
472-
firstOnly,
473-
nodeTypeNames,
474-
resolvedFields
475-
)
470+
// const siftResult /*: Array<IGatsbyNode> | null */ = filterWithSift(
471+
// filters,
472+
// firstOnly,
473+
// nodeTypeNames,
474+
// resolvedFields
475+
// )
476+
477+
// if (stats) {
478+
// if (!siftResult || siftResult.length === 0) {
479+
// stats.totalSiftHits++
480+
// }
481+
// }
476482

477483
if (stats) {
478-
if (!siftResult || siftResult.length === 0) {
479-
stats.totalSiftHits++
480-
}
484+
// to mean, "empty results"
485+
stats.totalSiftHits++
481486
}
482487

483-
return siftResult
488+
if (firstOnly) {
489+
return []
490+
}
491+
return null
484492
}
485493

486494
const filterToStats = (
@@ -512,6 +520,8 @@ const filterToStats = (
512520
* @returns {Array<IGatsbyNode> | null} Collection of results.
513521
* Collection will be limited to 1 if `firstOnly` is true
514522
*/
523+
// TODO: we will drop this entirely when removing all Sift code
524+
// eslint-disable-next-line no-unused-vars
515525
const filterWithSift = (filters, firstOnly, nodeTypeNames, resolvedFields) => {
516526
let nodes /*: IGatsbyNode[]*/ = []
517527
nodeTypeNames.forEach(typeName => addResolvedNodes(typeName, nodes))

‎packages/gatsby/src/schema/__tests__/node-model.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ describe(`NodeModel`, () => {
286286
})
287287
;[
288288
{ desc: `with cache`, cb: () /*:FiltersCache*/ => new Map() }, // Avoids sift for flat filters
289-
{ desc: `no cache`, cb: () => null }, // Always goes through sift
289+
// { desc: `no cache`, cb: () => null }, // Always goes through sift
290290
].forEach(({ desc, cb: createFiltersCache }) => {
291291
describe(`runQuery [${desc}]`, () => {
292292
it(`returns first result only`, async () => {
@@ -622,7 +622,7 @@ describe(`NodeModel`, () => {
622622
})
623623
;[
624624
{ desc: `with cache`, cb: () /*:FiltersCache*/ => new Map() }, // Avoids sift for flat filters
625-
{ desc: `no cache`, cb: () => null }, // Always goes through sift
625+
// { desc: `no cache`, cb: () => null }, // Always goes through sift
626626
].forEach(({ desc, cb: createFiltersCache }) => {
627627
it(`[${desc}] should not resolve prepared nodes more than once`, async () => {
628628
nodeModel.replaceFiltersCache(createFiltersCache())
@@ -835,7 +835,7 @@ describe(`NodeModel`, () => {
835835
})
836836
;[
837837
{ desc: `with cache`, cb: () => new Map() }, // Avoids sift
838-
{ desc: `no cache`, cb: () => null }, // Requires sift
838+
// { desc: `no cache`, cb: () => null }, // Requires sift
839839
].forEach(({ desc, cb: createFiltersCache }) => {
840840
describe(`[${desc}] Tracks nodes returned by queries`, () => {
841841
it(`Tracks objects when running query without filter`, async () => {

‎packages/gatsby/src/schema/__tests__/queries.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,10 @@ describe(`Query schema`, () => {
116116
query: {
117117
filter: {
118118
frontmatter: {
119-
authors: { email: { eq: source.email } },
120-
// authors: {
121-
// elemMatch: { email: { eq: source.email } },
122-
// },
119+
// authors: { email: { eq: source.email } },
120+
authors: {
121+
elemMatch: { email: { eq: source.email } },
122+
},
123123
},
124124
},
125125
},

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

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
const { runQuery: nodesQuery } = require(`../../db/nodes`)
2-
const { didLastFilterUseSift } = require(`../../redux/run-sift`)
2+
// const { didLastFilterUseSift } = require(`../../redux/run-sift`)
33
const { store } = require(`../../redux`)
44
const { actions } = require(`../../redux/actions`)
55

@@ -325,9 +325,9 @@ function resetDb(nodes) {
325325

326326
async function runQuery(
327327
queryArgs,
328-
filtersCache,
329-
nodes = makeNodesUneven(),
330-
alwaysSift = !filtersCache
328+
filtersCache = new Map(),
329+
nodes = makeNodesUneven()
330+
// alwaysSift = !filtersCache
331331
) {
332332
resetDb(nodes)
333333
const { sc, type: gqlType } = makeGqlType(nodes)
@@ -342,18 +342,18 @@ async function runQuery(
342342

343343
let result = await nodesQuery(args)
344344

345-
// If the filters cache was set, then the test expects fast filters to handle
346-
// the query and to skip Sift. If it's not set, then it can't use fast filters
347-
// Might revise this when adding tests where we don't expect fast filters to
348-
// support it yet..?
349-
if (result && result.length) {
350-
// Currently, empty results must go through Sift, so ignore those cases here
351-
if (alwaysSift) {
352-
expect(didLastFilterUseSift()).toBe(true)
353-
} else {
354-
expect(didLastFilterUseSift()).toBe(!filtersCache)
355-
}
356-
}
345+
// // If the filters cache was set, then the test expects fast filters to handle
346+
// // the query and to skip Sift. If it's not set, then it can't use fast filters
347+
// // Might revise this when adding tests where we don't expect fast filters to
348+
// // support it yet..?
349+
// if (result && result.length) {
350+
// // Currently, empty results must go through Sift, so ignore those cases here
351+
// if (alwaysSift) {
352+
// expect(didLastFilterUseSift()).toBe(true)
353+
// } else {
354+
// expect(didLastFilterUseSift()).toBe(!filtersCache)
355+
// }
356+
// }
357357

358358
return result
359359
}
@@ -399,7 +399,7 @@ it(`should use the cache argument`, async () => {
399399

400400
// Make sure to test fast filters (with cache) and Sift (without cache)
401401
;[
402-
{ desc: `without cache`, cb: () => null }, // Forces no cache, must use Sift
402+
// { desc: `no cache`, cb: () => null }, // Forces no cache, must use Sift
403403
{ desc: `with cache`, cb: () => new Map() },
404404
].forEach(({ desc, cb: createFiltersCache }) => {
405405
async function runFastFilter(filter) {

0 commit comments

Comments
 (0)