Skip to content

Commit 976dadb

Browse files
authored
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
1 parent 61c7392 commit 976dadb

File tree

1 file changed

+14
-5
lines changed

1 file changed

+14
-5
lines changed

src/index.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,17 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
8080
context.debug(() => `${blue("generated declarations")} for '${key}'`);
8181
}
8282

83+
/** common resolution check -- only resolve files that aren't declarations and pass `filter` */
84+
const shouldResolve = (id: string): boolean => {
85+
if (id.endsWith(".d.ts") || id.endsWith(".d.cts") || id.endsWith(".d.mts"))
86+
return false;
87+
88+
if (!filter(id))
89+
return false;
90+
91+
return true;
92+
}
93+
8394
/** to be called at the end of Rollup's build phase, before output generation */
8495
const buildDone = (): void =>
8596
{
@@ -207,10 +218,7 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
207218
if (!resolved)
208219
return;
209220

210-
if (resolved.endsWith(".d.ts"))
211-
return;
212-
213-
if (!filter(resolved))
221+
if (!shouldResolve(resolved))
214222
return;
215223

216224
cache.setDependency(resolved, importer);
@@ -277,7 +285,8 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
277285
// Rollup can't see these otherwise, because they are "emit-less" and produce no JS
278286
if (result.references && supportsThisLoad) {
279287
for (const ref of result.references) {
280-
if (!filter(ref))
288+
// 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
289+
if (!shouldResolve(ref))
281290
continue;
282291

283292
const module = await this.resolve(ref, id);

0 commit comments

Comments
 (0)