Adds Typings

[Silvenga]

Resolves #43.

I tested with the following permutations:

import EleventyFetch from "@11ty/eleventy-fetch";

// Buffer
const a = await EleventyFetch("", {

});

// unknown
const b = await EleventyFetch("", {
    type: "json"
});

// string
const c = await EleventyFetch("", {
    type: "text"
});

// any
const d = await EleventyFetch<any>("", {
    type: "json"
});

// type
const e = await EleventyFetch<{ test: string }>("", {
    type: "json"
});

// Failure
const f = await EleventyFetch<any>("", {
    type: "text"
});

// Failure
const g = await EleventyFetch<string>("", {
    type: "text"
});

// Buffer
const h = await EleventyFetch("");

This pr also exposes the types EleventyFetchOptions and FetchType. The default generic FetchType = "buffer" is a little wonky, but I couldn't find another way to represent default options.

[Zegnat]

This needs to be a true dependency, not just for development. Otherwise when you add eleventy-fetch to your normal dependencies, your package manager is unlikely to download @types/node and the type declaration file will become incomplete.

[Silvenga]

I don't like the idea of declaring this as a full dependency, as that would require all projects, regardless of type support, to pull a rather large type definition. Some bundlers will bundle all dependencies.

While, yes, native TS projects may include this as a dependency, and TS recommends shipping the types in the final bundle, this isn't a native TS project, and most consumers won't be typescript based.

[Silvenga]

This sums up my experiences: microsoft/types-publisher#81 (comment)

[Zegnat]

That’s an interesting sumup of the issue! But it does not seem to land at any specific conclusion 🤔

I am confused how the eleventy-fetch.d.ts is supposed to work if it depends on a type (from @types/node) that is not in my local environment. Surely Typescript is just going to tell me that the following code returns with a type any then?

eleventyFetch("", { type: "buffer" });

I do understand the issue of incompatible node types, conflicts with the parent, etc. I am now thinking this should maybe be a peerDependency, potentially set as "@types/node": "*". That way at least the package manager can inform the end-user that there is an unmet requirement when no node types have been loaded.

I think * is a perfectly valid version here, as the code is happy to accept any version of Node.js that supports Buffer.

[Silvenga]

I think having an error is better than telling typeless users that they are missing a peer dependency or pulling in a large dependency. Again, focusing on the primary user being a user of 11ty - which lacks types (types were accidently removed recently).

The whole, "don't let perfection prevent progress" idea. There aren't any perfect solutions here, so I'm optimizing for not "harming" existing users, while making TS users slightly better.

Another thing to consider, if a TS user needs the buffer api, wouldn't they already have @types/node installed in the consuming project?

[Zegnat]

How much completion should there be in the very first type declaration file? Note that eleventy-fetch.js adds a lot more to the module.exports than just the default function. Following some of the examples from the Typescript documentation on module exports what is required is a export = … statement that exports both the default function and everything else that was added to module.exports.

I think this is supposed to work something like this:

declare function eleventyFetch(url: string, options?: EleventyFetchOptions<"buffer">): Buffer;
declare function eleventyFetch(url: string, options?: EleventyFetchOptions<"text">): string;
declare function eleventyFetch(url: string, options?: EleventyFetchOptions<"json">): unknown;
declare function eleventyFetch<T extends any>(
	url: string,
	options?: EleventyFetchOptions<"json">,
): T;

declare namespace eleventyFetch {
	let concurrency: number;
	function queue<T extends any>(source: string, queueCallback: () => T): Promise<T> | undefined;
	const Util: { isFullUrl: (url: string | URL) => boolean };
	const RemoteAssetCache: RemoteAssetCacheConstructor;
	const AssetCache: AssetCacheConstructor;
}

export = eleventyFetch;

The functions are based on your start, I have then added the declarations for the other things configurable on the export.

For the Cache classes I have tried to follow the older Typescript documentation on static constructor vs instance types. So I am exposing the constructor interface, which will result in a normal interface to be created.

This is very much incomplete still, but the type would be something like:

interface AssetCacheConstructor {
	new (
		uniqueKey: string,
		cacheDirectory: string,
		options: EleventyFetchOptions<"text">,
	): AssetCacheInterface;
}
interface AssetCacheInterface {
	log(message: string): void;
	source: unknown;
	hash: unknown;
	cacheDirectory: string;
	readonly cacheFilename: string;
	readonly rootDir: string;
	readonly cachePath: string;
	readonly cache: unknown; // Actually from flatCache.load?!
	getDurationMs(duration: string): number;
	getCachedContentsPath(type: FetchType): string;
	ensureDir(): Promise<void>;
	save(contents: unknown, type: FetchType): Promise<void>;
	getCachedContents(type: FetchType): Promise<any>;
	_backwardsCompatibilityGetCachedValue(type: FetchType): any;
	getCachedValue(): Promise<any>;
	isCacheValid(duration: string): boolean;
	readonly cachedObject: any;
	needsToFetch(duration: string): boolean;
	fetch(options: EleventyFetchOptions): unknown;
}

interface RemoteAssetCacheConstructor {
	new (
		url: string | URL,
		cacheDirectory: string,
		options: EleventyFetchOptions<"text">,
	): RemoteAssetCacheInterface;
}
interface RemoteAssetCacheInterface extends AssetCacheInterface {}

There is definitely still a lot of additional work left to do here. But maybe it can be split over multiple PRs?

@zachleat do you have any thought on how you would like types to get introduced, other than PRs being welcome?

[Silvenga]

@Zegnat I don't think it would be wise to increase the scope here. CommonJS is on the way out, so I would expect this library to eventually update to ES modules. Supporting CommonJS style API's through typed namespaces has always been wonky.

[yamadashy]

It appears that the return types for these functions should be wrapped in Promise.
This is consistent with how PQueue's add method is typed, which returns a Promise as seen here: https://github.com/sindresorhus/p-queue/blob/main/source/index.ts#L234