fix(gatsby-link): Do not crash in unit tests when globals are undefined

[blainekasten]

Description

This allows gatsby-link to be used in unit tests where __BASE_PATH_ and __PREFIX_PATH_ do not exist.

Related Issues

Fixes #24789

[gatsby-cloud-staging]

Your pull request can be previewed in Gatsby Cloud: https://build-5e9e04b7-6c8a-40ff-9f35-d885c85a1827.staging-previews.gtsb.io

[wardpeet]

Are we sure we want this? Adding more code just to support tests..

We tell people here how to set things up https://www.gatsbyjs.org/docs/unit-testing/#2-creating-a-configuration-file-for-jest

[blainekasten]

@wardpeet totally understand your thought here. Maybe I should wrap those checks in NODE_ENV for tests. That way it gets stripped out in prod builds since our prod builds are guaranteed to have those values.

I personally don't think that having docs is enough. From a consumers perspective this would be super frustrating to deal with. The Link might be deep within the tree, and all you get as an obtuse error about an undefined variable you've never heard of. Seeking this down to write a unit test would be quite encumbering. I think we should make this easier.

Does wrapping in a NODE_ENV for tests satisfy you?

[ascorbic]

Nested ternaries are really hard to read. How about

Edit: oops

[ascorbic]

Ironically, the ternary confused me so I misunderstood what was happening! I see what you're doing, but I'd still rather replafe the ternary with an early return.

[pvdz]

In the context of this discussion; I prefer to use === over !== if there is an else branch unless there's a clear reason to show the !== branch first. I think it reads better as inverting adds to the mental overhead of parsing code. I agree with Matt that this one is a little hairy. Sometimes it is what it is.

I would consider to ignore the production check (what purpose does that serve?) since it currently assumes the var exists in production anyways, so typeof __PATH_PREFIX__ === 'undefined' should be false regardless. So I'd suggest this instead.

Suggested change

	const getGlobalPathPrefix = () =>
	process.env.NODE_ENV !== `production`
	? typeof __PATH_PREFIX__ !== `undefined`
	? __PATH_PREFIX__
	: undefined
	: __PATH_PREFIX__
	const getGlobalPathPrefix = () =>
	// Prevent reference error; the `__PATH_PREFIX__` binding may not exist in tests
	typeof __PATH_PREFIX__ === `undefined` ? undefined : __PATH_PREFIX__

[pieh]

Are we sure we want this? Adding more code just to support tests..

Not just tests, but other 3rd party tools like storybook also need similar treatment (as evidenced by #25643)

[pvdz]

still lgtm and apparently Ward also gave his stamp on slack

You could consider simplifying the ternary, in that case just update and force merge afaic.