New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(typescript)!: don't resolve filtered files (#1267) #1310
Conversation
This is actually a breaking change, because the error being thrown has changed signature for that scenario. I'd also like to see a test with a fixture containing the |
@lukastaegert getting some new CI errors. could they be ralated to your recent work? |
Yes, you are correct about the changed error signature (though, Regarding the test, I'm not sure how I should cover the Regarding the CI problems: I could not run the tests without some changes (e.g. |
Sorry about the CI issues, if you rebase against latest master and run |
Thanks Lukas, after the rebase everything worked fine again. @shellscape Based on my previous comment, do you have any suggestions on how we can proceed with this issue? |
b353836
to
3038271
Compare
My only thought is to brute force a unit test of that line, where that condition causes the error that was changed. You don't necessarily need to have TS recognize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for being patient on this one. I'd like one more maintainer to put eyes on this before we merge.
Rollup Plugin Name:
typescript
This PR contains:
Are tests included?
Breaking Changes?
If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.
List any relevant issue numbers:
#1267
Description
BREAKING CHANGES: The signature of the error being thrown has changed.
This should fix the issue #1267, which is caused by the use of
order: "post"
as part of #1245. It will prevent the typescript plugin from resolving files which are not included by its provided configuration/options. Instead thenode-resolve
plugin will again handle these cases, which is able to correctly handle thebrowser
config parameter. The same fix has been done for therollup-plugin-typescrip2
here: ezolenko/rollup-plugin-typescript2@f6db596