sveltejs / kit

web development, streamlined
https://svelte.dev/docs/kit
MIT License
18.73k stars 1.95k forks source link

Add thrown objects (i.e. Redirect, HttpError) to @sveltejs/kit package exports #10062

Open nicholas-wilcox opened 1 year ago

nicholas-wilcox commented 1 year ago

Describe the problem

I would like to write unit tests for my server hooks. Specifically, I want to add test cases that make assertions about SvelteKit's "control" objects that get thrown by redirect, error, and fail.

What I can do now, using Vitest as it comes preconfigured by create-svelte, is something like the following:

// src/hooks.server.test.ts
import type { RequestEvent } from '@sveltejs/kit';
import { describe, expect, it, vi } from 'vitest';
import * as hooks from './hooks.server';
import { mockRequestEvent } from '$lib/mocks/request'; // My own module

describe('redirect hook', () => {
  it('throws redirect', async () => {
    event = mockRequestEvent() // My own function
    const resolve = vi.fn(async (_event: RequestEvent) => new Response());
    const test = async () => hooks.redirectHook({ event, resolve });
    await expect(test).rejects.toThrowError();
    expect(resolve).toHaveBeenCalledTimes(0);
  });
});

However, any usage of the optional argument of toThrowError() will result in an error, because the objects that are thrown aren't actually errors.

What I would like to to is write custom tests using expect.extend that use the instanceof operator to check that the thrown object is the correct type.

The weird thing about this is that TypeScript lets me write an import statement that looks like it imports the Redirect class, not the type. Unfortunately, the symbol evaluates to undefined during runtime, which makes code like actual instanceof Redirect or Redirect.prototype throw runtime errors.

In other words, this is valid code for a fresh SvelteKit application made with create-svelte:

// src/hooks.server.ts
import type { Handle, RequestEvent } from '@sveltejs/kit';
import { error, redirect, Redirect } from '@sveltejs/kit';

export const handle = (async ({ event, resolve }) => {
  console.log(Redirect); // prints "undefined"
  return await resolve(event);
}) satisfies Handle;

However, trying to print Redirect.prototype throws a TypeError.

Describe the proposed solution

Export Redirect, HttpError, and ActionFailure in https://github.com/sveltejs/kit/blob/master/packages/kit/src/exports/index.js.

Adding an export statement manually in node_modules/@sveltejs/kit/src/exports/index.js prevents the errors described above.

Alternatives considered

I only want access to these objects for testing purpose, so I can also use Vitest's mocking features to create a mock for @sveltejs/kit. However, this solution requires me to redeclare the classes in my own module:

// src/setup-test.ts -- Added as a setup file in vite.config.ts
import { vi } from 'vitest';
import { Redirect, HttpError, error, redirect } from '$lib/mocks/control';

vi.doMock('@sveltejs/kit', () => ({ Redirect, redirect }));

expect.extend({
  toThrowRedirect(actual: Redirect, status: number, location?: string) {
    if (!(actual instanceof Redirect)) {
      return {
        pass: false,
        message: () => 'Object is not Redirect type',
      };
    }

    // Individual checks on status and location omitted for brevity
  }
})

Now, the thrown objects will have accessible prototypes, but they will be from the classes declared in my own module.

Importance

nice to have

Additional Information

I'm not sure if the quirk that prevents a type-check error qualifies as a bug.

Also, because we aren't supposed to catch these objects in our application code, I can't think of a reason that anyone would want to access Redirect.prototype outside of testing.

nicholas-wilcox commented 1 year ago

I'm aware of the "grotesque" and "shameful" hack that relates to these objects. However, I don't fully understand what's going on here.

I don't think the way this module is loaded has anything to do with the issues I'm experiencing, but I also don't know if my proposed solution will break the hack.

elliott-with-the-longest-name-on-github commented 1 year ago

I advocated for exposing these from the start, for exactly this reason, but at the time @Rich-Harris was against it. I can't remember why, and I can't find the conversations around it, so maybe it's time to open the discussion back up?