fix: handle all type-only imports by piping TS imports (#406)

* fix: handle all type-only imports by piping TS imports

- `result.references` is populated by `ts.preProcessFile`; i.e. this is TS discovering all imports, instead of Rollup
  - TS's imports include type-only files as TS understands those (whereas they aren't emitted in the JS for Rollup to see, since, well, they produce no JS)
  - so we can pipe all these through Rollup's `this.resolve` and `this.load` to make them go through Rollup's `resolveId` -> `load` -> `transform` hooks
    - this makes sure that other plugins on the chain get to resolve/transform them as well
    - and it makes sure that we run the same code that we run on all other files on type-only ones too
      - for instance: adding declarations, type-checking, setting them as deps in the cache graph, etc
      - yay recursion!
        - also add check for circular references b/c of this recursion (which Rollup docs confirm is necessary, per in-line comment)
    - and Rollup ensures that there is no perf penalty if a regular file is processed this way either, as it won't save the hook results when it appears in JS (i.e. Rollup's module graph)
      - we are checking more files though, so that in and of itself means potential slowdown for better correctness

- add a test for this that uses a `tsconfig` `files` array, ensuring that the `include` workaround won't cover these type-only files
  - this test fails without the new code added to `index` in this commit
  - also add another file, `type-only-import-import`, to the `no-errors` fixture to ensure that we're not just checking imports one level deep, and actually going through type-only imports of type-only imports as well
    - the declaration check for this will fail if type-only imports are not handled recursively
      - an initial version of this fix that I had that didn't call `this.load` failed this check

- refactor(test): make the integration tests more resilient to output ordering changes
  - due to the eager calls to `this.load`, the ordering of declaration and declaration map outputs in the bundle changed
    - and bc TS's default ordering of imports seems to differ from Rollup's
  - note that this only changed the order of the "bundle output" object -- which Rollup doesn't guarantee ordering of anyway
    - all files are still in the bundle output and are still written to disk
    - for example, the `watch` tests did not rely on this ordering and as such did not need to change due to the ordering change
  - create a `findName` helper that will search the `output` array instead, ensuring that most ordering does not matter
    - we do still rely on `output[0]` being the bundled JS (ESM) file, however

- refactor(test): go through a `files` array for tests that check for multiple files instead of listing out each individual check
  - this makes the tests more resilient to fixture changes as well (i.e. addition / deletion of files)
  - create `no-errors.ts` that exports a list of files for this fixture
    - didn't need to do the same for `errors.ts` as of yet; may do so in the future though

* move type-only recursion to after declarations are added to the `declarations` dict

- preserve some ordering and simplify future debugging

- also fix lint issue, `let modules` -> `const modules`
  - I previously changed it (while WIP), but now it's static/never reassigned, so can use `const`

* rewrite the core logic with a `for...of` loop instead

- simpler to follow than `map` + `filter` + `Promise.all`
  - might(?) be faster without `Promise.all` as well as more can happen async without waiting
    - (I'm not totally sure of the low-level implementation of async to know for sure though)

* add comment about normalization

__tests__/integration/errors.spec.ts

@@ -5,7 +5,7 @@ import { normalizePath as normalize } from "@rollup/pluginutils";
 import * as fs from "fs-extra";
 
 import { RPT2Options } from "../../src/index";
-import * as helpers from "./helpers";
+import { findName, genBundle as genBundleH } from "./helpers";
 
 // increase timeout to 15s for whole file since CI occassionally timed out -- these are integration and cache tests, so longer timeout is warranted
 jest.setTimeout(15000);
@@ -21,7 +21,7 @@ afterAll(async () => {
 
 async function genBundle(relInput: string, extraOpts?: RPT2Options, onwarn?: Mock) {
   const input = local(`fixtures/errors/${relInput}`);
-  return helpers.genBundle({
+  return genBundleH({
     input,
     tsconfig: local("fixtures/errors/tsconfig.json"),
     testDir,
@@ -41,10 +41,11 @@ test("integration - semantic error - abortOnError: false / check: false", async
   const { output: output2 } = await genBundle("semantic.ts", { check: false }, onwarn);
   expect(output).toEqual(output2);
 
-  expect(output[0].fileName).toEqual("index.js");
-  expect(output[1].fileName).toEqual("semantic.d.ts");
-  expect(output[2].fileName).toEqual("semantic.d.ts.map");
-  expect(output.length).toEqual(3); // no other files
+  const files = ["index.js", "semantic.d.ts", "semantic.d.ts.map"];
+  files.forEach(file => {
+    expect(findName(output, file)).toBeTruthy();
+  });
+  expect(output.length).toEqual(files.length); // no other files
   expect(onwarn).toBeCalledTimes(1);
 });
 
@@ -80,11 +81,10 @@ test("integration - type-only import error - abortOnError: false / check: false"
   }, onwarn);
   expect(output).toEqual(output2);
 
-  expect(output[0].fileName).toEqual("index.js");
-  expect(output[1].fileName).toEqual("import-type-error.d.ts");
-  expect(output[2].fileName).toEqual("import-type-error.d.ts.map");
-  expect(output[3].fileName).toEqual("type-only-import-with-error.d.ts");
-  expect(output[4].fileName).toEqual("type-only-import-with-error.d.ts.map");
-  expect(output.length).toEqual(5); // no other files
+  const files = ["index.js", "import-type-error.d.ts", "import-type-error.d.ts.map", "type-only-import-with-error.d.ts.map", "type-only-import-with-error.d.ts.map"];
+  files.forEach(file => {
+    expect(findName(output, file)).toBeTruthy();
+  });
+  expect(output.length).toEqual(files.length); // no other files
   expect(onwarn).toBeCalledTimes(1);
 });

__tests__/integration/fixtures/no-errors.ts

@@ -0,0 +1,11 @@
+export const filesArr = [
+  "index.js",
+  "index.d.ts",
+  "index.d.ts.map",
+  "some-import.d.ts",
+  "some-import.d.ts.map",
+  "type-only-import.d.ts",
+  "type-only-import.d.ts.map",
+  "type-only-import-import.d.ts",
+  "type-only-import-import.d.ts.map",
+];

__tests__/integration/fixtures/no-errors/index.ts

@@ -6,6 +6,6 @@ import { difference } from "./some-import";
 export const diff2 = difference; // add an alias so that this file has to change when the import does (to help test cache invalidation etc)
 
 export { difference } from "./some-import"
-export type { num } from "./type-only-import"
+export type { num, num2 } from "./type-only-import"
 
 export { identity } from "./some-js-import"

__tests__/integration/fixtures/no-errors/type-only-import-import.ts

@@ -0,0 +1 @@
+export type numb = number;

__tests__/integration/fixtures/no-errors/type-only-import.ts

@@ -1 +1,4 @@
+import type { numb } from "./type-only-import-import";
+
 export type num = number;
+export type num2 = numb;

__tests__/integration/helpers.ts

@@ -1,4 +1,4 @@
-import { rollup, watch, RollupOptions, OutputOptions, OutputAsset, RollupWatcher } from "rollup";
+import { rollup, watch, RollupOptions, OutputOptions, RollupOutput, OutputAsset, RollupWatcher } from "rollup";
 import * as path from "path";
 
 import rpt2, { RPT2Options } from "../../src/index";
@@ -72,3 +72,8 @@ export async function watchBundle (inputArgs: Params) {
   await watchEnd(watcher); // wait for build to end before returning, similar to genBundle
   return watcher;
 }
+
+export function findName (output: RollupOutput['output'], name: string) {
+  // type-cast to simplify type-checking -- [0] is always chunk, rest are always asset in our case
+  return output.find(file => file.fileName === name) as OutputAsset;
+}

__tests__/integration/no-errors.spec.ts

@@ -1,11 +1,11 @@
 import { jest, afterAll, test, expect } from "@jest/globals";
 import * as path from "path";
 import * as fs from "fs-extra";
-import { OutputAsset } from "rollup";
 import { normalizePath as normalize } from "@rollup/pluginutils";
 
 import { RPT2Options } from "../../src/index";
-import * as helpers from "./helpers";
+import { filesArr } from "./fixtures/no-errors";
+import { findName, genBundle as genBundleH } from "./helpers";
 
 // increase timeout to 15s for whole file since CI occassionally timed out -- these are integration and cache tests, so longer timeout is warranted
 jest.setTimeout(15000);
@@ -17,7 +17,7 @@ const fixtureDir = local("fixtures/no-errors");
 afterAll(() => fs.remove(testDir));
 
 async function genBundle(relInput: string, extraOpts?: RPT2Options) {
-  return helpers.genBundle({
+  return genBundleH({
     input: `${fixtureDir}/${relInput}`,
     tsconfig: `${fixtureDir}/tsconfig.json`,
     testDir,
@@ -33,36 +33,45 @@ test("integration - no errors", async () => {
   const { output: outputWithCache } = await genBundle("index.ts");
   expect(output).toEqual(outputWithCache);
 
-  expect(output[0].fileName).toEqual("index.js");
-  expect(output[1].fileName).toEqual("index.d.ts");
-  expect(output[2].fileName).toEqual("index.d.ts.map");
-  expect(output[3].fileName).toEqual("some-import.d.ts");
-  expect(output[4].fileName).toEqual("some-import.d.ts.map");
-  expect(output[5].fileName).toEqual("type-only-import.d.ts");
-  expect(output[6].fileName).toEqual("type-only-import.d.ts.map");
-  expect(output.length).toEqual(7); // no other files
+  const files = filesArr;
+  files.forEach(file => {
+    expect(findName(output, file)).toBeTruthy();
+  });
+  expect(output.length).toEqual(files.length); // no other files
 
   // JS file should be bundled by Rollup, even though rpt2 does not resolve it (as Rollup natively understands ESM)
   expect(output[0].code).toEqual(expect.stringContaining("identity"));
 
   // declaration map sources should be correctly remapped (and not point to placeholder dir, c.f. https://github.com/ezolenko/rollup-plugin-typescript2/pull/221)
-  const decMapSources = JSON.parse((output[2] as OutputAsset).source as string).sources;
+  const decMap = findName(output, "index.d.ts.map");
+  const decMapSources = JSON.parse(decMap.source as string).sources;
   const decRelPath = normalize(path.relative(`${testDir}/dist`, `${fixtureDir}/index.ts`));
   expect(decMapSources).toEqual([decRelPath]);
 });
 
+test("integration - no errors - using files list", async () => {
+  const { output } = await genBundle("index.ts", { tsconfigOverride: { files: ["index.ts"] } });
+
+  // should still have the type-only import and type-only import import!
+  const files = filesArr;
+  files.forEach(file => {
+    expect(findName(output, file)).toBeTruthy();
+  });
+  expect(output.length).toEqual(files.length); // no other files
+});
+
 test("integration - no errors - no declaration maps", async () => {
   const noDeclarationMaps = { compilerOptions: { declarationMap: false } };
   const { output } = await genBundle("index.ts", {
     tsconfigOverride: noDeclarationMaps,
     clean: true,
   });
 
-  expect(output[0].fileName).toEqual("index.js");
-  expect(output[1].fileName).toEqual("index.d.ts");
-  expect(output[2].fileName).toEqual("some-import.d.ts");
-  expect(output[3].fileName).toEqual("type-only-import.d.ts");
-  expect(output.length).toEqual(4); // no other files
+  const files = filesArr.filter(file => !file.endsWith(".d.ts.map"));
+  files.forEach(file => {
+    expect(findName(output, file)).toBeTruthy();
+  });
+  expect(output.length).toEqual(files.length); // no other files
 });
 
 
@@ -88,12 +97,15 @@ test("integration - no errors - allowJs + emitDeclarationOnly", async () => {
     },
   });
 
-  expect(output[0].fileName).toEqual("index.js");
-  expect(output[1].fileName).toEqual("some-js-import.d.ts");
-  expect(output[2].fileName).toEqual("some-js-import.d.ts.map");
-  expect(output.length).toEqual(3); // no other files
+  const files = ["index.js", "some-js-import.d.ts", "some-js-import.d.ts.map"];
+  files.forEach(file => {
+    expect(findName(output, file)).toBeTruthy();
+  });
+  expect(output.length).toEqual(files.length); // no other files
 
   expect(output[0].code).toEqual(expect.stringContaining("identity"));
   expect(output[0].code).not.toEqual(expect.stringContaining("sum")); // no TS files included
-  expect("source" in output[1] && output[1].source).toEqual(expect.stringContaining("identity"));
+
+  const dec = findName(output, "some-js-import.d.ts");
+  expect(dec.source).toEqual(expect.stringContaining("identity"));
 });

__tests__/integration/watch.spec.ts

@@ -3,6 +3,7 @@ import * as path from "path";
 import * as fs from "fs-extra";
 
 import { RPT2Options } from "../../src/index";
+import { filesArr } from "./fixtures/no-errors";
 import * as helpers from "./helpers";
 
 // increase timeout to 15s for whole file since CI occassionally timed out -- these are integration and cache tests, so longer timeout is warranted
@@ -36,15 +37,6 @@ test("integration - watch", async () => {
   const distPath = `${testDir}/dist/index.js`;
   const decPath = `${distDir}/index.d.ts`;
   const decMapPath = `${decPath}.map`;
-  const filesArr = [
-    "index.js",
-    "index.d.ts",
-    "index.d.ts.map",
-    "some-import.d.ts",
-    "some-import.d.ts.map",
-    "type-only-import.d.ts",
-    "type-only-import.d.ts.map",
-  ];
 
   const watcher = await watchBundle(srcPath);
 

src/index.ts

@@ -33,6 +33,7 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
 	let documentRegistry: tsTypes.DocumentRegistry; // keep the same DocumentRegistry between watch cycles
 	let cache: TsCache;
 	let noErrors = true;
+	let transformedFiles: Set<string>;
 	const declarations: { [name: string]: { type: tsTypes.OutputFile; map?: tsTypes.OutputFile } } = {};
 	const checkedFiles = new Set<string>();
 
@@ -150,6 +151,9 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
 
 			cache = new TsCache(pluginOptions.clean, pluginOptions.objectHashIgnoreUnknownHack, servicesHost, pluginOptions.cacheRoot, parsedConfig.options, rollupOptions, parsedConfig.fileNames, context);
 
+			// reset transformedFiles Set on each watch cycle
+			transformedFiles = new Set<string>();
+
 			// printing compiler option errors
 			if (pluginOptions.check) {
 				const diagnostics = convertDiagnostic("options", service.getCompilerOptionsDiagnostics());
@@ -203,8 +207,10 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
 			return null;
 		},
 
-		transform(code, id)
+		async transform(code, id)
 		{
+			transformedFiles.add(id); // note: this does not need normalization as we only compare Rollup <-> Rollup, and not Rollup <-> TS
+
 			if (!filter(id))
 				return undefined;
 
@@ -245,6 +251,22 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
 
 			addDeclaration(id, result);
 
+			// handle all type-only imports by resolving + loading all of TS's references
+			// Rollup can't see these otherwise, because they are "emit-less" and produce no JS
+			if (result.references) {
+				for (const ref of result.references) {
+					if (ref.endsWith(".d.ts"))
+						continue;
+
+					const module = await this.resolve(ref, id);
+					if (!module || transformedFiles.has(module.id)) // check for circular references (per https://rollupjs.org/guide/en/#thisload)
+						continue;
+
+					// wait for all to be loaded (otherwise, as this is async, some may end up only loading after `generateBundle`)
+					await this.load({id: module.id});
+				}
+			}
+
 			// if a user sets this compilerOption, they probably want another plugin (e.g. Babel, ESBuild) to transform their TS instead, while rpt2 just type-checks and/or outputs declarations
 			// note that result.code is non-existent if emitDeclarationOnly per https://github.com/ezolenko/rollup-plugin-typescript2/issues/268
 			if (parsedConfig.options.emitDeclarationOnly)