From 976dadb53a62e32958f949b72a1c16093af2a329 Mon Sep 17 00:00:00 2001
From: Anton Gilgur <4970083+agilgur5@users.noreply.github.com>
Date: Mon, 10 Jul 2023 01:25:12 -0400
Subject: [PATCH] fix: hardcode declaration extension check (#456)

* fix: preemptively `filter` type-only files to workaround Rollup bug

- basically, right now, the addition of `this.load` on all `references` is causing Rollup to error out on JSON files
  - specifically, this is impacting `configPlugin` usage (i.e. `rollup.config.ts`), where previously one didn't need `@rollup/plugin-json`, but now it is erroring out without it
  - I tracked this down to be because of `this.load` specifically
  - to avoid this and similar such issues, we can preemptively `filter` out files before calling `this.resolve` / `this.load`, which should end up `exclude`ing JSON files and any other non-rpt2 files
    - this should also make it a bit more efficient to skip some recursion
    - and non-rpt2 files shouldn't include any type-only files

- confirmed that this change fixes the error
  - and that the type-only tests still pass

* refactor: use a common function for resolution checks

- since the same logic is used in `resolveId` and these _should_ be equivalent

- in the future, we might want to add more common logic to this function, e.g. `getAllReferences` removes `undefined` and uses `moduleNameResolver` as well, similar to `resolveId`
  - may not be so easy, so TBD
    - for instance, even moving the `undefined` check into the func required adding a type guard, as the compiler wasn't quite able to infer that passing the func meant it was not `undefined`

* feat: also ignore `.d.cts` and `.d.mts` file extensions

- support newer TS extensions

- rpt2 should _always_ ignore declarations
  - regardless of the `exclude`; as in, if a user accidentally removes declarations in an override, rpt2 should still not directly read declarations
    - as they are normally read ambiently by TS and not _directly_ by Rollup or TS

* fix comment -- there's more reasons why we shouldn't resolve all references

- we don't `return false` in `resolveId`, so any new file that wasn't previously in Rollup's pipeline _must_ be resolved
  - `return` just defers to the next plugin, so, for a declaration, it eventually causes Rollup to try and fail to resolve on its own, giving an `Unexptected token` error message
  - but we _don't_ want to `return false` in `resolveId` if they _intentionally_ imported a declaration for some reason (e.g. if they're going to transform it in some way)
    - if we did `return false`, no other plugin could process either
  - so as a result, we should just never call `this.resolve()` on anything we don't expect to be able to resolve
    - i.e. don't add anything new to the pipeline that we don't resolve ourselves
---
 src/index.ts | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/src/index.ts b/src/index.ts
index cb1f7d15..694b15a2 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -80,6 +80,17 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
 		context.debug(() => `${blue("generated declarations")} for '${key}'`);
 	}
 
+	/** common resolution check -- only resolve files that aren't declarations and pass `filter` */
+	const shouldResolve = (id: string): boolean => {
+		if (id.endsWith(".d.ts") || id.endsWith(".d.cts") || id.endsWith(".d.mts"))
+			return false;
+
+		if (!filter(id))
+			return false;
+
+		return true;
+	}
+
 	/** to be called at the end of Rollup's build phase, before output generation */
 	const buildDone = (): void =>
 	{
@@ -207,10 +218,7 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
 			if (!resolved)
 				return;
 
-			if (resolved.endsWith(".d.ts"))
-				return;
-
-			if (!filter(resolved))
+			if (!shouldResolve(resolved))
 				return;
 
 			cache.setDependency(resolved, importer);
@@ -277,7 +285,8 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
 			// Rollup can't see these otherwise, because they are "emit-less" and produce no JS
 			if (result.references && supportsThisLoad) {
 				for (const ref of result.references) {
-					if (!filter(ref))
+					// pre-emptively filter out files that we don't resolve ourselves (e.g. declarations). don't add new files to Rollup's pipeline if we can't resolve them
+					if (!shouldResolve(ref))
 						continue;
 
 					const module = await this.resolve(ref, id);