fix(string-interpolation): operation headers not interpolated when req.headers is a HeaderMap

[pmrotule]

Description

Considering how to set dynamic header values: https://the-guild.dev/graphql/mesh/docs/guides/headers

This PR allows the mesh config operationHeaders to be a function that returns the headers object. It takes the resolverData as first argument so we can have access to the context.req.headers. This fixes the issues where the headers object is a HeaderMap (you need to call headers.get('key')) which is not interpolated as expected:

Fixes #5226

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• This change requires a documentation update

Sandbox

🚀 See it in action by checking out the fixed branch of the issue reproduction repo. The changes of this PR has been published on npm under @pmrotule/graphql-mesh-string-interpolation@0.4.3-beta.2.

How Has This Been Tested?

It has been tested in the issue reproduction repo and our real GraphQL Mesh integration (not tested on production yet). We still need to add test in this PR. Any guidance around tests would be appreciated since I haven't found any test regarding string interpolation.

Test Environment:

• OS: MacOS Monterey 12.6.1
• @graphql-mesh/string-interpolation: 0.4.2 (latest)
• NodeJS: 18.14.0
• express: 4.18.2 (latest)
• @apollo/server: 4.5.0 (latest)

Checklist:

• I have followed the
  CONTRIBUTING doc and the
  style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests and linter rules pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

Further comments

I made the changes in the string-interpolation package even though it doesn't perform any string interpolation. It might be better to define it elsewhere, but I would love your feedback on that one.

We also started to get warnings from the JSON Schema validation of the mesh config, but I'm not sure where to update this (is it based on the Typescript types?).

[changeset-bot]

⚠️ No Changeset found

Latest commit: b48b0f1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[Urigo]

thank you @pmrotule !
could you also add a test that covers this?

[ardatan]

I'm not sure if your change is related to the issue. That line is related to the operationHeaders input not handling method calls inside interpolated strings.

[pmrotule]

thank you @pmrotule ! could you also add a test that covers this?

@Urigo I can, but it would be great if you could recommend me a place to write it since the string-interpolation package doesn't have tests yet. Can I simply add it to the graphql handler?

I'm not sure if your change is related to the issue. That line is related to the operationHeaders input not handling method calls inside interpolated strings.

@ardatan It depends how you see it. It's a change that offers a new way of setting the operationHeaders and that new way doesn't have the issue anymore. The PR could also be considered a new feature instead of a fix.

In my opinion, setting the operationHeaders with a function is more reliable than doing string interpolation with regular expressions, but no matter what I think, it's already a step in the right direction if graphql-mesh can provide a way to set the operationHeaders that doesn't have the issue. We could keep the issue open until it is also fixed in the string interpolation. What do you think?

[ardatan]

I guess your issue has been fixed and I think that feature you mentioned needs to be discussed because I am not sure about mixing JS Code and configuration.
Closing this for now.