Anton Gilgur 560ed8dac7
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
2022-08-30 13:07:13 -06:00
..