Skip to content

Commit

Permalink
fix: don't skip resolving files imported by other plugins (ezolenko#365)
Browse files Browse the repository at this point in the history
- previously the `allImportedFiles` Set was _basically_ used to skip any files that were imported by other plugins
  - it only filtered out on `importer` (not `importee`)
  - this is a bit of a problem though, as other plugins that create JS code, or `allowJs` JS code, or heck, even regular JS code that Rollup processes without plugins, may actually import TS files
    - and Rollup's plugin system is designed to handle that scenario, so rpt2 actually _should_ resolve those files
      - notably, rpt2 already _transforms_ those files, it just didn't resolve them
        - which is why using `@rollup/plugin-node-resolve` with a `.ts` extension solved this as a workaround
          - the file would be resolved by `node-resolve` then actually transformed by rpt2
          - but rpt2 should resolve TS files itself, `node-resolve` shouldn't be necessary
    - this caused the issue that extensionless imports from such files wouldn't get resolved to TS files and may cause Rollup to error
      - in particular, this popped up with extensionless imports of TS files from Svelte files

- `resolveId` is actually the last remaining place where `allImportedFiles` was used
  - so after removing it from here, we can just remove it entirely
    - which should be a small optimization

- for reference:
  - `allImportedFiles` would be any `include`d files as well as TS files that have been transformed (at this point in the build) and their references
    - this is a pretty gigantic list as is, so I'm actually not sure that removing this is actually that big of a de-opt
      - and it only affects mixed TS / non-TS codebases too
  - this seems to have been added in b0a0ecb, but that commit doesn't actually seem to fix the issue it references
    - see my root cause analyses in the issues
  • Loading branch information
agilgur5 authored Jun 24, 2022
1 parent 4ea7f10 commit b0e3922
Showing 1 changed file with 0 additions and 12 deletions.
12 changes: 0 additions & 12 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
let service: tsTypes.LanguageService;
let noErrors = true;
const declarations: { [name: string]: { type: tsTypes.OutputFile; map?: tsTypes.OutputFile } } = {};
const allImportedFiles = new Set<string>();

let _cache: TsCache;
const cache = (): TsCache =>
Expand Down Expand Up @@ -103,8 +102,6 @@ const typescript: PluginImpl<RPT2Options> = (options) =>

if (generateRound === 0)
{
parsedConfig.fileNames.map(allImportedFiles.add, allImportedFiles);

context.info(`typescript version: ${tsModule.version}`);
context.info(`tslib version: ${tslibVersion}`);
if (this.meta)
Expand Down Expand Up @@ -156,10 +153,6 @@ const typescript: PluginImpl<RPT2Options> = (options) =>

importer = normalize(importer);

// avoiding trying to resolve ids for things imported from files unrelated to this plugin
if (!allImportedFiles.has(importer))
return;

// TODO: use module resolution cache
const result = tsModule.nodeModuleNameResolver(importee, importer, parsedConfig.options, tsModule.sys);
let resolved = result.resolvedModule?.resolvedFileName;
Expand Down Expand Up @@ -197,8 +190,6 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
if (!filter(id))
return undefined;

allImportedFiles.add(normalize(id));

const contextWrapper = new RollupContext(pluginOptions.verbosity, pluginOptions.abortOnError, this, "rpt2: ");

const snapshot = servicesHost.setSnapshot(id, code);
Expand Down Expand Up @@ -229,9 +220,6 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
if (!result)
return undefined;

if (result.references)
result.references.map(normalize).map(allImportedFiles.add, allImportedFiles);

if (watchMode && this.addWatchFile && result.references)
{
if (tsConfigPath)
Expand Down

0 comments on commit b0e3922

Please sign in to comment.