request-policy-exchange behavior differs between dev & production

[Akkuma]

Describe the bug

The exchange has this basic logic:

const processIncomingResults = (result: OperationResult): void => {
  const meta = result.operation.context.meta;
  const isMiss = !meta || meta.cacheOutcome === 'miss';
  if (isMiss) {
    operations.set(result.operation.key, new Date().getTime());
  }
};

meta does not exist in production (as mentioned #3439) resulting in this always being viewed as a miss. This will keep a request alive much longer than anticipated and won't be the same behavior between dev and production.

Reproduction

Is this needed here?

Urql version

4.0.6, core 4.2.2

Validations

• I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
• Read the docs.
• Follow our Code of Conduct

[kitten]

No reproduction needed here indeed! Nice catch. We haven't kept track of this exchange as much as we should have, so i'll get on that and will see what we can do.

It might be worth evaluating at the same time whether this exchange should be expanded to include more logic.

[Akkuma]

No reproduction needed here indeed! Nice catch. We haven't kept track of this exchange as much as we should have, so i'll get on that and will see what we can do.

It might be worth evaluating at the same time whether this exchange should be expanded to include more logic.

Locally I modified this exchange to use localStorage rather than be in memory, so that the ttl persists across multiple sessions. What I've done locally is remove a key when it should be upgraded and my set will only set if there is no key. This keeps it from impacting the intended ttl and doesn't require meta to exist either:

const get = (key: string | number): number => {
	const val = localStorage.getItem(`${storageNamespace}${key}`);
	return val ? parseInt(val, 10) : 0;
};

const set = (key: string | number): void => {
	if (localStorage.getItem(`${storageNamespace}${key}`)) return;

	localStorage.setItem(`${storageNamespace}${key}`, `${Date.now()}`);
};

const remove = (key: string | number): void => {
	localStorage.removeItem(`${storageNamespace}${key}`);
};

// relevant get & remove code
const currentTime = Date.now();
const lastOccurrence = get(operation.key);
if (currentTime - lastOccurrence > TTL && (!options.shouldUpgrade || options.shouldUpgrade(operation))) {
  remove(operation.key);
  return makeOperation(operation.kind, operation, {
	...operation.context,
	requestPolicy: 'cache-and-network',
  });
} 

I could open up a PR to support an optional storage get/set/remove like the above allowing for any storage mechanism.

[alpavlove]

Hey, folks. I know this was closed long ago, but in the @urql/core distributive files, I still see this condition that prevents setting meta cache outcome in the production environment. I'm using the version 5.0.4. You can see it here. Can we fix it?

result = {
    ...result,
    operation: process.env.NODE_ENV !== 'production' ? addMetadata(operation, {
      cacheOutcome: cachedResult ? 'hit' : 'miss'
    }) : operation
  };

[alpavlove]

I think I have found the culprit - the babel transformer and fixed it in this PR.