fix: Remove errant export of GetByRoleMatcher, fixing type checking in some TS configurations #575
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What:
Follow-up to #143 , and the issue reported at #574.
Mixing
export type
withexport = matchers
can throw off TypeScript in some configurations. We do not need to export the matcher as a separate type, since the argument types are available in the matcher already.I will admit, I do not know the exact combination of configuration that causes this issue. I think it would be desirable to include a test case for it, to ensure that we do not break it accidentally in the future.
The types are doing quite a lot of heavy lifting (types for explicit extend, types on globals/declaration merging, plus being "external" to the source), so perhaps we are not testing all the combinations. I am a bit surprised that we did not get this error in the existing tests, the double export sounds conceptually invalid. The issue at #574 has a reproduction, but paring it down to the size of the existing tests might be best.
Anyway, in terms of compatibility, I am confident that this is the best approach. By avoiding the extra
export
, we do not confuse TypeScript, and we are back in the previous status quo (exports = matchers
only).Why:
Avoiding unintended type changes, in some environments.
How:
By reverting what the TypeScript error was pointing at:
Checklist:
I think this is ready to be merged, but the lack of an inverse test case is bothering me 馃槄
Please let me know if I can help in any way, and I'll give it a shot!