Skip to content

chore: Improve types #1061

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

Merged
merged 3 commits into from
Feb 16, 2023
Merged

chore: Improve types #1061

merged 3 commits into from
Feb 16, 2023

Conversation

TimothyJones
Copy link
Contributor

  • npm run dist works locally (this will run tests, lint and build)
  • Commit messages are ready to go in the changelog (see below for details)
  • PR template filled in (see below for details)

PR Template

This fixes #1054 by widening the type so that the V3 matchers no longer need to be called with something that can be assigned to AnyTemplate. Since this is a widening of the type, it is not a breaking change.

I also removed all references to AnyTemplate and added tsdoc so that it is deprecated. It's still exported, but just not used anywhere. It can be removed in a future major release.

While I was there, I refactored the types out of matchers.ts into types.ts for readability. These are now re-exported by matchers.ts so that any (inappropriate) deep links will still work, and anyone importing the type from MatchersV3.Matcher will still work.

I also added a TODO comment that in some future release we'll need to rename the interfaces that have the same names in the V2 and V3 matchers. This affects maintainers, as the current situation will result in confusing types, but I don't think it affects users.

Please don't squash merge this, as the commit messages contain the details for the release notes.

…as the side effect that functions, Dates and other inappropriate types can now be passed to matchers, and the benefit that people using interfaces don't get spurious errors. Fixes pact-foundation#1054
@TimothyJones
Copy link
Contributor Author

The build is failing because of unrelated reasons. The examples need to be updated (looks like mocha-pact needs updating too).

}

if (Object.prototype.toString.call(input) === '[object Array]') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what on earth was going on here in the first place 😬

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old school JS

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Array.isArray is “reasonably” new

@mefellows
Copy link
Member

Hmm this is a new failure, I've not seen this one before but it seems like it exists in master so entropy has done its thing https://github.com/pact-foundation/pact-js/actions/runs/4190675102/jobs/7264320560#step:4:1477

I'm going to merge and sort out the other build after, and any issues with the types if they are discovered.

Thank you, this is much appreciated!

@mefellows mefellows merged commit aba0595 into pact-foundation:master Feb 16, 2023
@TimothyJones
Copy link
Contributor Author

That failure is because mocha-pact’s peer dependency is set to 9.x, so later versions of npm are pulling in the older one (and should also be failing on npm install- I guess npm ci doesn’t fail like install does, or something).

npm is getting better at peer dependencies, so we need to be more careful about them being correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Question: How to use TypeScript interfaces with matchers
2 participants