fastify / fastify-vite

Fastify plugin for Vite integration.
MIT License
849 stars 71 forks source link

fix: explicitly export named type #128

Closed MrBazlow closed 7 months ago

MrBazlow commented 8 months ago

Description

There is no named export for the plugin, encountered this when trying to dynamically import the plugin into an ESM file. This change copies the export style of @fastify/cookie. I presume that this was not caught by tests because something (Vitest? Node?) is helping with CJS compatibility that masks the problem when running tests.

I have not included tests for this for two reasons, first because it does not seem to be tested for in various other Fastify packages and secondly because the test I produced feels very hacky. I have included it below.

index.test.mjs

import { expect, test } from 'vitest'

test('esm - should import plugin dynamically with default and named', async () => {
  const FastifyVite = await import('./index.js')
  expect(FastifyVite.default).toBeDefined()
  expect(FastifyVite.fastifyVite).toBeDefined()
})

vitest.config.js

import { defineConfig } from 'vitest/config'

export default defineConfig({
  root: process.cwd(),
  base: process.cwd(),
  test: {
    threads: false,
    deps: {
      interopDefault: false,
      moduleDirectories: ['.']
    }
  }
})

Needing two flags to recreate this phenomenon and the potential side effects that may cause is why I'm on the side of caution and haven't included a test for this. The cost might outweigh the benefits.

Checklist

netlify[bot] commented 8 months ago

Deploy Preview for agitated-mahavira-26f8f9 canceled.

Name Link
Latest commit 63911b2343045bc781e70a723834294feda13416
Latest deploy log https://app.netlify.com/sites/agitated-mahavira-26f8f9/deploys/659782a2de2f8f000875040e