Skip to content

add custom properties to request object #1653

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 5 commits into from
May 8, 2024
Merged

add custom properties to request object #1653

merged 5 commits into from
May 8, 2024

Conversation

FreeAoi
Copy link
Contributor

@FreeAoi FreeAoi commented May 3, 2024

Changes

What does this PR change? Link to any related issue(s).
This pull request lets request object attach custom properties like next prop on nextjs environment.

image
image

How to Review

It should works
How can a reviewer review your changes? What should be kept in mind for this review?

Checklist

  • Unit tests updated
  • docs/ updated (if necessary)
  • pnpm run update:examples run (only applicable for openapi-typescript)

Copy link

changeset-bot bot commented May 3, 2024

🦋 Changeset detected

Latest commit: 592b0f6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
openapi-fetch Patch

Not sure what this means? Click here to learn what changesets are.

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

@drwpow
Copy link
Contributor

drwpow commented May 3, 2024

Thanks for adding. Would it be possible to add a test, to ensure the correct usage is working as intended?

@FreeAoi
Copy link
Contributor Author

FreeAoi commented May 3, 2024

Sure

@FreeAoi
Copy link
Contributor Author

FreeAoi commented May 3, 2024

How I should do the unit test?

@drwpow
Copy link
Contributor

drwpow commented May 3, 2024

How I should do the unit test?

Inside the main test suite, inside middleware, I’d just write a test that creates example middleware and adds the arbitrary property.

Then from the MSW side, assert that the server received the request with that additional property on it (see the can modify request test) as an example).

The bonus part of adding a test is the types will be validated, too. The code you wrote looks good, and it looks like it achieves the desired result! But adding a test guarantees it works as-advertised, and doesn’t throw any errors in TypeScript either.

@FreeAoi
Copy link
Contributor Author

FreeAoi commented May 4, 2024

Should be ready, check it out and let me know your thoughts

Copy link
Contributor

@drwpow drwpow left a comment

Choose a reason for hiding this comment

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

This is great, thank you! Fantastic test!

@drwpow drwpow merged commit 4f4253a into openapi-ts:main May 8, 2024
6 checks passed
@drwpow
Copy link
Contributor

drwpow commented May 16, 2024

🤔 I’m wondering if we need to add docs for this; this may just be an invisible feature. I’m going to release this, and will skip docs for now, but also open to a PR adding docs if it’s helpful.

@FreeAoi
Copy link
Contributor Author

FreeAoi commented May 17, 2024

Hi, I think write docs for this is not necessary because It is not common (it could be a bad practice in fact) monkeypatching request object.

This pr it's principal objective is let request object support custom properties in cases where frameworks like nextjs modified the request interface

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