Skip to content
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

feat(operationalParameter): Default to config headers for header params #1865

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

optikalefx
Copy link

If OpenAPI has paramaters that will be sent as headers, allow configuration to set those headers for the entire client.

This PR generates the following typescript

 public listWorkflows({
        instanceId,
        amazonAdvertisingApiClientId = this.httpRequest.config.HEADERS['Amazon-Advertising-API-ClientId'],
        amazonAdvertisingApiAdvertiserId = this.httpRequest.config.HEADERS['Amazon-Advertising-API-AdvertiserId'],
        amazonAdvertisingApiMarketplaceId = this.httpRequest.config.HEADERS['Amazon-Advertising-API-MarketplaceId'],
        nextToken,
        limit,
    }

Which allows a user to use the library as follows

const myClient = new MyClient({
		TOKEN: token,
		HEADERS: {
			'Amazon-Advertising-API-ClientId': AMAZON_ADS_CLIENT_ID,
			'Amazon-Advertising-API-AdvertiserId': AMAZON_ADS_ADVERTISING_ID,
			'Amazon-Advertising-API-MarketplaceId': AMAZON_ADS_US_MARKETPLACE
		}
	});

And then method calls no longer require you to pass these headers every time. This is a welcome change to verbosity.

Feedback is welcome.

If OpenAPI has paramaters that will be sent as headers, allow configuration to set those headers for the entire client.
@optikalefx
Copy link
Author

To the maintainers, there are at least 3 ways I could think of to implement this.

  1. What you see in this PR, which is simply overriding the default in getOperationParameter.ts. This is the least code changed, but doesn't make the most sense overall.

  2. Making the change in getModelDefault.ts. Which makes more sense, but requires us to have access to parameter not schema, in order to access in. I did try this change first, to pass parameter instead of definition: OpenApiSchema. However this requires changing getModel and getOperationParameter in quite a number of places.

  3. Making a new partial (this was my first approach) called {{>defaultParameters}} and doing this work there in the hbs. However, we don't love logic in the hbs, and {{{default}}} already handles the case where ?: has to be applied vs just : So it was a bit of repeat logic that default already implements.

At any rate, I went with the least changes necessary for now, but happy to discuss the other approaches. I could make 3 PRs to look at if you want to, but before I write a test, I wanted to check with you on approach.

Thanks!

@jordanshatford
Copy link

Check out our (previously forked) version of this package https://github.com/hey-api/openapi-ts. We have just recently added support for setting request/response interceptors. This may fit as an alternative solution to what you are trying to accomplish. Please check it out if you'd like, and create any issues if you run into problems.

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.

2 participants