ezolenko / rollup-plugin-typescript2

Rollup plugin for typescript with compiler errors.
MIT License
822 stars 71 forks source link

fix: `filter` "missed" declarations as well #347

Closed agilgur5 closed 2 years ago

agilgur5 commented 2 years ago

Summary

Only output declarations for files that pass the filter check so there isn't a mismatch between JS being excluded, but DTS declarations still being output.

Details

agilgur5 commented 2 years ago

Huh, so I was looking into allImportedFiles because it causes #283 (see root cause analysis in https://github.com/ezolenko/rollup-plugin-typescript2/issues/283#issuecomment-1163899409), and I found that the commit that adds it also removed the filter that was already there on this line of the diff.

So this PR undoes that line. And with #176 and #346, basically that entire commit is undone....

Considering #283 is caused by the follow-up commit, it seems like the entire allImportedFiles Set may in fact be erroneous and have caused several regressions... 😕

It seems to have been created for #162 and https://github.com/ezolenko/rollup-plugin-typescript2/issues/136#issuecomment-515148624 :

In any case, the main impact of allImportedFiles was undone in #176 anyway. So removing the resolution piece for #283 shouldn't have much impact, other than being a bugfix / regression fix.

agilgur5 commented 2 years ago

Yeaaaa, did some more digging and per https://github.com/ezolenko/rollup-plugin-typescript2/issues/283#issuecomment-1163930973 , allImportedFiles from #162 seems to have been entirely erroneous and a source of at least 5 regressions in the past few years (some that were quite common).

The last two fixes for those regressions, this PR and #365, have yet to be released either, so like big OOF on how many regressions for how many years that's caused 😕 😕