0

A test in our frankly very complicated React app is taking ~1000 times longer to run than expected, and I'm trying to pin down where/why.

I started by manually calling console.time('name of test') in the test file, and then manually dotting console.timeLog('name of test', 'did a thing') around the application in all the places that were called and seemed likely to cause slow downs.

I noticed a lot of these places were inside React hooks - they aren't slow themselves, but were helping me see how long it took for coffee to get there.

I decided I needed to write a monkey patch in a Jest setupFilesAfterEnv file for logging when React hooks callback are called, and what with (for this example, I'll use useEffect)

const React = require('react');

let timeTestName = null;
let doTimeLog = false;
let prevUseEffect;

beforeAll(() => {
  ({ timeTestName } = global);
  const prevUseEffect = React.useEffect;
  React.useEffect = (cb, deps) => {
    if(doTimeLog && timeTestName && Array.isArray(deps) && !__filename.includes('node_modules')){
      console.timeLog(timeTestName, `Use Effect called with ${JSON.stringify(deps, null, 2)}`): // log useEffect being called with timer
    }
    prevUseEffect(cb, deps);
  }
});

beforeEach(() => {
  const { testPath } = expect.getState();  
  if(testPath.endsWith(`${timeTestName}.test.js`)) {
    doTimeLog = true;
    console.time(timeTestName); // start timer
  } else {
    doTimerLog = false;
  }
});

afterEach(() => {
  doTimerLog = false;
  console.log(testToTimeName); // end timer
});

afterAll(() => {
  React.useEffect = prevUseEffect;
})

However what I really want as well is the variable names in the dependency list, which I cannot get (at least not without changing non-test code).

One thought I had was using a Jest transformer to make all the arrays into objects so I preserve the variable names as keys; something like:


module.exports = {
  process(sourceText) {
    return {
      code: `convertSquareToCurly(sourceText)`,
    };
  },
};

module.exports = {
  transform: {
    'matchDepList':
      '<rootDir>/deplistTransformer.js',
  },
};

So during tests only my useEffects become:

useEffect(() => foo(a, b, c), { a, b, c})

(I will handle this new object in my monkey patch)

I'm reasonably confident I can make the above work.

Then I in my monkey patch I can call console.log(Object.entries(deps)); and prevUseEffect(cb, Object.values(deps));.

However if I'm concerned that calling Object.values will cause the hook's callback to always be called.

I haven't been able to try the transformer yet, but I don't want to waste time writing it if passing Object.values won't work in place of the untransformed dependency list.

Is there another way to monkey patch and get variable names, or would this work?

AncientSwordRage
  • 7,086
  • 19
  • 90
  • 173
  • 1
    What's the _context_ - why do you think you need this? Interfering with interfaces you don't own is generally a test/design smell. – jonrsharpe May 20 '22 at 18:03
  • What's wrong with just passing the object inside that dependency array? I don't see a scenario where this wouldn't work. Keep things standard man as per React interfaces. – Mosia Thabo May 20 '22 at 18:07
  • As a safe option, you could consider just working with dependency array indexes. At least those you have in runtime without additional transpilers. – Igor Loskutov May 20 '22 at 18:09
  • @jonrsharpe one of our tests is timing out in our very large react app. I'm planning on this either being temporary or only switched on by a flag passed to `--global` – AncientSwordRage May 20 '22 at 18:10
  • @MosiaThabo I don't want to touch non-test code – AncientSwordRage May 20 '22 at 18:11
  • 1
    @MosiaThabo do you mean like `[{ foo, bar, baz }]` instead of `[foo, bar, baz]`? That would be a new, unequal object on every render and therefore trigger the effect every time, you might as well have no deps at that point. – jonrsharpe May 20 '22 at 18:11
  • @IgorLoskutov that's definitely a backup – AncientSwordRage May 20 '22 at 18:11
  • The array from `Object.values` would still contain references to the original values, so it wouldn't cause the effect to be triggered unnecessarily. But without the actual context this is likely an XY problem with a totally different solution. – jonrsharpe May 20 '22 at 18:13
  • [{ foo, bar, baz }] would actually work if you do them with `[useMemo(() => ({foo, bar, baz}), [foo, bar, baz])]` although that would be super weird in code – Igor Loskutov May 20 '22 at 18:14
  • For debugging your performance, would that help you? https://github.com/simbathesailor/use-what-changed – Igor Loskutov May 20 '22 at 18:15
  • Don't do this you can mock React with spyOn with a jest function – Filip Cordas May 20 '22 at 18:16
  • @jonrsharpe no more like converting `[a, b, c]` into `{a, b, c}` – AncientSwordRage May 20 '22 at 18:17
  • 1
    @AncientSwordRage what they said was _"object inside the dependency array"_. React's expecting something with a length and maybe a 0th, 1th, etc. value, so you certainly can't just pass an object. I guess you could pass something that has _both_ though, indexed **and** named values. But again this is probably all in entirely the wrong direction. – jonrsharpe May 20 '22 at 18:20
  • @FilipCordas that's very much deeper into the "just because you _can_, doesn't mean you _should_" space - don't mock what you don't own. – jonrsharpe May 20 '22 at 18:20
  • @jonrsharpe Read the documentation on spyon it' created for this use case he is doing something he shouldn't be doing, he shouldn't be 'monkey patching' anything inside react object and use the jest functionality if he needs it. – Filip Cordas May 20 '22 at 18:28
  • @jonrsharpe I'm hoping if I only transform my *use* of useEffect, and pass an array into the original (via `prevUseEffexr`) I can avoid issues. I'll edit with more detail presently – AncientSwordRage May 20 '22 at 18:31
  • @FilipCordas can you call jest.spyOn inside a setupFilesAfterEnv file? – AncientSwordRage May 20 '22 at 19:03
  • @jonrsharpe added context. May still be doing the wrong thing, but I'm curious. – AncientSwordRage May 20 '22 at 19:04
  • 1
    While you've already been up and down this tree, I can't help but feel that this is excessive. I would wager it's considerably easier to globally find and replace `useEffect()` with a-la [`useEffectDebugger()`](https://stackoverflow.com/a/59843241/1470607) and go from there if you really want to see all hooks firing. – Etheryte May 20 '22 at 19:15
  • @Etheryte how does that tell me the timing of each hook firing? – AncientSwordRage May 20 '22 at 19:53
  • You can add timing information to it similar to your approach. The main suggestion here is to avoid all of this unrelated work with transformers, spies etc, which while technically interesting, is not required to solve the problem you're facing. – Etheryte May 20 '22 at 20:29
  • @Etheryte it's not a bad idea. I was partly inspired by https://www.npmjs.com/package/jest-prop-type-error when I started out, I didn't think it'd be this gross – AncientSwordRage May 20 '22 at 21:51

0 Answers0