Add configurable fetcher for import metadata

[ije]

Summary

• add an injectable fetcher (setFetcher) so import metadata requests are not hard-wired to globalThis.fetch
• export setFetcher from the import map API and declare it in the published type definitions
• update Bun tests to mock network metadata responses via setFetcher instead of spyOn(globalThis, \"fetch\")

Test plan

• Run bun test src/add.test.ts

Made with Cursor

[Copilot]

Pull request overview

This PR adds a configurable setFetcher function that allows callers to inject a custom HTTP fetcher instead of being hard-wired to globalThis.fetch. This is useful for testing and for environments where globalThis.fetch may not be the desired transport. Additionally, the PR includes several refactors converting template literals to string concatenation, and extracts a trimLeadingStar helper.

Changes:

• Adds setFetcher export to src/add.ts, src/importmap.ts, and types/index.d.ts
• Updates src/add.test.ts to use setFetcher instead of spyOn(globalThis, "fetch") for mocking network calls
• Refactors template literals to string concatenation throughout src/add.ts, extracts trimLeadingStar helper, and modifies getCdnOrigin trailing-slash removal

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/add.ts	Adds Fetcher type, setFetcher function, replaces bare fetch with injectable fetcher; also refactors template literals and getCdnOrigin
src/importmap.ts	Re-exports setFetcher from add.ts
types/index.d.ts	Declares setFetcher in public type definitions
src/add.test.ts	Replaces spyOn(globalThis, "fetch") with setFetcher-based mocking wrapped in try/finally

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The error variable in a catch clause is typed as unknown in TypeScript (with useUnknownInCatchVariables, which is on by default in strict mode and in Bun's TypeScript). Accessing error.message directly will cause a TypeScript type error. The previous code avoided this by not capturing the error variable at all (catch {), and instead included bodyText.slice(0, 200) in the error message, which provided useful debugging context. The new code should either cast the error to Error using error instanceof Error ? error.message : String(error), or restore the previous bodyText.slice(0, 200) approach.

Suggested change

	throw new Error("invalid meta response from " + url + ": " + error.message);
	const message = error instanceof Error ? error.message : String(error);
	throw new Error("invalid meta response from " + url + ": " + message);

[Copilot]

The original implementation used cdn.replace(/\/+$/, "") which strips one or more consecutive trailing slashes. The new implementation uses cdn.endsWith("/") and cdn.slice(0, -1), which only removes a single trailing slash. A CDN URL like https://my-cdn.example.com/// would previously be normalized to https://my-cdn.example.com, but with the new code it becomes https://my-cdn.example.com//, causing malformed request URLs. The old regex-based approach should be preserved, or the slice should be applied in a loop (or use trimEnd-based approach) to match the original behavior.

Suggested change

	if (cdn.endsWith("/")) {
	// remove trailing slash
	return cdn.slice(0, -1);
	}
	return cdn;
	// remove one or more trailing slashes to normalize the origin
	return cdn.replace(/\/+$/, "");

[Copilot]

The JSDoc @param tag says @param fetcher but the actual parameter is named f. This mismatch means documentation tooling will not correctly associate the parameter description with the actual parameter. The parameter should be renamed from f to fetcher to match the docstring and to be more descriptive.

[Copilot]

In both mocked tests, setFetcher is called before the try/finally block that restores the fetcher. In the "warns on unmet peer dependency" test, setFetcher (line 78) and spyOn(console, "warn") (line 111) both run before the try block (line 112). If spyOn throws, the mock fetcher is never restored and will leak into subsequent tests. Similarly, in the "removes scope specifiers..." test, setFetcher (line 138) runs before the try block (line 157). The setFetcher call should be moved inside the try block (or placed at the very top of it), just as spyOn is, so that the finally cleanup always runs after the setup.