chore(project-config): Cache results of common functions

[Josh-Walker-GM]

We have two functions getPaths and getConfigPath which are good candidates for caching. Their output is deterministic from the input and they are called often.

The original comment for getPaths mentioned using a proxy. I did not understanding the motivation behind that as we're exporting a function which returns an object and not just a paths object itself. I might just not be seeing the obvious though.

I had thought to freeze the paths object to prevent unintended mutations but decided not to fix a problem that doesn't exist.

[Tobbe]

Is computing the paths map actually slow? Like, are commands noticeably faster with the caching?

The comment about the Proxy I think is more about the second part of that same comment. The part about making it lazy. I.e. only compute the path for something when it's first accessed. But honestly I think that'd be slower overall than just doing what we have now. String concatenation can't really be that slow, can it?

[Josh-Walker-GM]

Ahh I see what is meant by the proxy now, thanks!

I didn't do any benchmarks on this. I would bet the benefit is quite small and not likely to be noticeable. I would still argue it does no harm to cache this given we know it's typically called more than once and it's essentially a constant.

[jtoar]

I'll just add that this is mainly a refactor, since we already make this optimization. We're trying to deduplicate the code here as we make more CLI packages:

redwood/packages/cli-packages/storybook/src/lib/project.ts

Lines 11 to 19 in 708902d

	export const _getPaths = () => {
	try {
	return getRedwoodPaths()
	} catch (e) {
	console.error(c.error((e as Error).message))
	process.exit(1)
	}
	}
	export const getPaths = memoize(_getPaths)

This was from the CLI here:

redwood/packages/cli/src/lib/index.js

Lines 197 to 209 in 708902d

	/**
	* This wraps the core version of getPaths into something that catches the exception
	* and displays a helpful error message.
	*/
	export const _getPaths = () => {
	try {
	return getRedwoodPaths()
	} catch (e) {
	console.error(c.error(e.message))
	process.exit(1)
	}
	}
	export const getPaths = memoize(_getPaths)

I'm not sure if we ever did the benchmarks before making this optimization, they'd still be good to do, but this an optimization we already make.

[dac09]

Just a thought - very often caching and memoisation in the CLI would have very little effect IMO. This is because CLI executions tend to me short running processes (the exceptions being dev, serve).

Which means anything cached in memory gets discarded as soon as the process has finished execution! IMHO, if we really wanted to do, the safest and least complicated option would be to just use the memoize.

[Josh-Walker-GM]

Thanks Danny. As Dom highlighted this was really just to upstream what we already do in the cli lib. It can't really hurt to cache these results even if it is just for a short time for a cli command.