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

refactor: Upgraded to h3 => v1.8.0 dependency in nitro-cors. #6

Merged
merged 1 commit into from
Aug 28, 2023

Conversation

michealroberts
Copy link
Owner

refactor: Upgraded to h3 => v1.8.0 dependency in nitro-cors.

Includes refactor to utilise the new EventHandlerRequest type from h3 v1.8.0 rather than any.

Includes refactor to deprecate the getMethod() utility in favour of event.method with h3 v1.8.0.

@michealroberts michealroberts added the enhancement New feature or request label Aug 26, 2023
@michealroberts michealroberts self-assigned this Aug 26, 2023
src/corsEventHandler.ts Outdated Show resolved Hide resolved
src/corsEventHandler.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@danielroe danielroe left a comment

Choose a reason for hiding this comment

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

Looks good to me. If you are able to verify inferred types are working well, that would be great. (Could maybe add a type fixture too just to confirm but that could be beyond the scope.)

@michealroberts
Copy link
Owner Author

Looks good to me. If you are able to verify inferred types are working well, that would be great. (Could maybe add a type fixture too just to confirm but that could be beyond the scope.)

Hey @danielroe , this would be cool.

I've taken a look at attempting to mock the H3 Event to check types, I need to mock the ServerResponse, but have had varying levels of success, do you know how this could be mocked?

I've tried this:

it('should return the correct types when specified', async () => {
      const handler = defineCORSEventHandler(
        async event => {
          return {
            cors: true
          }
        },
        {
          origin: '*',
          methods: '*'
        }
      )

      const req = new IncomingMessage(new Socket())

      const res = await handler(createEvent(req, {} as H3Event['res']))

      expectTypeOf(res).toEqualTypeOf<{
        cors: boolean
      }>()
    })

But the test is failing to append response headers (obviously, as it is dummy mocked).

Any ideas? 🤔

@danielroe
Copy link
Collaborator

For type testing, it's sufficient to have a file in which the utility is used - and then run a typechecking step. The file doesn't need to be run. See https://vitest.dev/guide/testing-types.html for more info.

@michealroberts
Copy link
Owner Author

For type testing, it's sufficient to have a file in which the utility is used - and then run a typechecking step. The file doesn't need to be run. See https://vitest.dev/guide/testing-types.html for more info.

Ah yes, quite right, makes sense. I've updated with a few small tests with verify everything is working well.

refactor: Upgraded to h3 => v1.8.0 dependency in nitro-cors.
@michealroberts michealroberts merged commit 877d98c into main Aug 28, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants