fix(projectify): optimize replaceProjectIdToken performance

[surbhigarg92]

This PR optimizes the core replaceProjectIdToken utility function for maximum performance, memory efficiency, and safety.

By moving to an optimized Selective-Write In-Place Mutation design and restructuring control flow, we eliminate exponential recursive redundancy on arrays, bypass unnecessary checks on primitive values, and prevent any write operations on unmodified object/array elements.

This keeps memory allocations at absolute zero (avoiding GC ticks) and allows frozen arrays and objects to be traversed safely without throwing TypeErrors as long as they do not contain placeholders to replace.

[gemini-code-assist]

Code Review

This pull request refactors the replaceProjectIdToken utility to optimize performance and adds tests for handling values without placeholders. The review feedback highlights two issues with the new implementation: mutating input arrays in-place (which is a regression from the original .map() implementation and can cause side effects or crash on frozen arrays) and performing unnecessary write operations on object properties. The reviewer suggested using a copy-on-write strategy for arrays and checking for actual changes before writing to object properties.

[surbhigarg92]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the replaceProjectIdToken function to optimize performance by avoiding unnecessary allocations when no placeholders are present. It also adds tests for frozen arrays and objects. However, mutating the input array in-place is a breaking change compared to the original implementation (which used .map()) and will throw a TypeError if a frozen array contains a placeholder. A copy-on-write approach is suggested to safely preserve backward compatibility and prevent runtime errors.

[gemini-code-assist]

Mutating the input array in-place is a breaking change compared to the original implementation, which used .map() to return a new array. If a caller passes an array and expects it to remain unmodified, this in-place mutation will introduce unexpected side effects. Furthermore, if a frozen array contains a placeholder, attempting to mutate it in-place will throw a TypeError at runtime.

To preserve the performance benefits of avoiding allocations when no placeholders are present, while maintaining safety and backward compatibility, we can use a Copy-on-Write approach. We only clone the array if we actually detect a modified element.

Suggested change

	if (Array.isArray(value)) {
	for (let i = 0; i < value.length; i++) {
	const original = value[i];
	const processed = replaceProjectIdToken(original, projectId);
	if (processed !== original) {
	value[i] = processed;
	}
	}
	return value;
	}
	if (Array.isArray(value)) {
	let cloned: any[] | null = null;
	for (let i = 0; i < value.length; i++) {
	const original = value[i];
	const processed = replaceProjectIdToken(original, projectId);
	if (processed !== original) {
	if (!cloned) {
	cloned = [...value];
	}
	cloned[i] = processed;
	}
	}
	return cloned || value;
	}

[surbhigarg92]

The original implementation was already mutating values in-place for all nested objects

// Original implementation mutated objects in-place:
for (const opt in value) {
  if (value.hasOwnProperty(opt)) {
    value[opt] = replaceProjectIdToken(value[opt], projectId);
  }
}

To address the concern about frozen arrays/objects without triggering new allocations, we implemented a Selective-Write strategy.

const original = value[i];
const processed = replaceProjectIdToken(original, projectId);
if (processed !== original) {
  value[i] = processed; // Only writes if a placeholder was actually found & changed!
}

[pearigee]

I think this could be simplified using more modern syntax:

for (const key of Object.keys(value)) {
    value[key] = replaceProjectIdToken(value[key], projectId);
}

I don't think we need the additional if (processed !== original) because this function has already been modified to return the references in place?

[surbhigarg92]

1. Object.keys(value) allocates a brand new array of strings , this triggers garbage collection and degrades speed compared to for...in.

2. if (processed !== original): selective-write check! - Without this check, the code will unconditionally attempt to write to value[key]. That means if a frozen object is passed (even one with no placeholders to replace), it will immediately crash with a TypeError. This check was intentionally added,

Earlier, this method was indirectly safe when receiving frozen arrays because the original implementation used .map(), which safely cloned the reference. Since we migrated off .map() to achieve better performance speedup via in-place mutation, we engineered the if (original !== processed) check to preserve that exact historical frozen-structure compatibility. Without it, the unconditional assignment in the Object.keys() snippet would trigger an immediate TypeError in strict mode on frozen structures, even when no replacements are made.

[pearigee]

Should we combine this check for Buffer and Stream with the other checks above for null and non-object primitives?

// Early return for null, primitives (i.e. booleans, numbers), Buffers, and Streams. These are non-traversable leaf nodes.
// Note this must come after the string check.
if (
    value === null ||
    typeof value !== 'object' ||
    value instanceof Buffer ||
    value instanceof Stream
) {
    return value;
}

[surbhigarg92]

I dont mind doing this change, but I was simply trying to micro-optimize here.

Chances of value having null, object or array is really high compared to Buffer or Stream. So I was trying to avoid this check value instanceof Buffer || value instanceof Stream for little later.