Skip to content

Conversation

@valkolovos
Copy link
Contributor

@valkolovos valkolovos commented May 20, 2025

  • 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)

This PR adds support for AsynchronousMessages to the V4 Pact

@YOU54F YOU54F requested review from YOU54F and mefellows May 30, 2025 12:03
@YOU54F
Copy link
Member

YOU54F commented May 30, 2025

Thanks for this @valkolovos 🚀

I've popped myself and Matt on as reviewers and will aim to test it out early next week.

Looks good from a quick eyeball.

We will probably need to revisit the messaging docs, just to ensure they are correct and if any additions are required off the back of this change, they are included

https://github.com/pact-foundation/pact-js/blob/master/docs/messages.md

but that can hold off a bit until we've done a technical review.

@YOU54F
Copy link
Member

YOU54F commented Jun 24, 2025

Added an example to the pact-workshop-message sample.

pact-foundation/pact-workshop-message@main...valkolovos-feat/issue-1186_v4_async_messages

I think we need to support the withMetadata function for parity, and think about how its passed in the execute block, is it a 2nd param for metadata, or is it an object that has .message and .metadata

@mefellows
Copy link
Member

OK, I've taken a review of this today. Thanks very much. It definitely works as is, however we need to think about some of the extensibility angles (including the plugin interface).

Some background

With the introduction of V4, including plugins, the interface (including the use of FFI) got a bit more complicated. After some experimentation and feedback, we landed on a type-state builder approach, which meant that you could only call methods after pre-requisite methods were called - i.e. the interface itself attempted to address validation errors, rather than relying on lots of internal state handling and guards.

In particular, you'll note this pattern is used to discriminate between regular and plugin-based interactions which have different setup requirements.

With this in mind, there are a few changes required to be incorporated here, primarily for consistency with the rest of the V4 interface:

  1. As Yousaf mentioned, we should get the metadata added to the interface (minor)
  2. We need to handle non-JSON payloads in messages (minor)
  3. We need to separate the plugin from the regular interface (this is where the API differences will mostly appear)

Most of those types exist in the other PR anyway, but your PR includes the reification of content.

Plan

  1. I've been incorporating your changes (and rebased onto latest) in this branch: master...feat/v4-asynchronous-messages (which had a bunch of other type improvements / cleanups that were overdue anyway)
  2. This will require a little bit of cleaning/review of the exposed interface and types (it might need some sub-packaging)
  3. I'll update the docs as well
  4. (optional) Lastly, the reifyMessage method is a bit of a wonky FFI implementation and has some problems (particularly with synchronous message). I left it in because it's bit of work to do it properly. I will see if I can incorporate those changes in now, in case it may require an interface change

I'll add you as a co-author to the commits.

@valkolovos
Copy link
Contributor Author

Thanks @mefellows! I originally planned to add the "withMetadata" functionality and just hadn't had time to do that yet. Do you still want me to get that done or are we good for now?

@mefellows
Copy link
Member

As discussed, I'm going to close this in favour of the rebased changes here: #1520

@mefellows mefellows closed this Jul 8, 2025
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.

3 participants