`module` field takes precedence over `exports` in package.json

posva commented 1 year ago

Describe the bug

Vite seems to prioritize the module field instead of the exports like node does when importing from a library.

I found this problem while using Firebase with Nuxt: it failed during dev but worked after building the project for production and serving the app with node. These are the other issues I found

Reproduction

https://stackblitz.com/edit/vitejs-vite-csrnaz

Steps to reproduce

npm i + npm run dev -> outputs module
node main.js -> outputs exports

It should always output exports

System Info

na

Used Package Manager

npm

Logs

No response

Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to vuejs/core instead.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
[X] The provided reproduction is a minimal reproducible example of the bug.

bluwy commented 1 year ago

Oh this seems like the same issue I was fixing in https://github.com/vitejs/vite/pull/11234, but was reverted as it caused some issue with Vue + vite-node + Histoire (discussion). It seems like Vue expects the module to take precedence over exports (?)

posva commented 1 year ago

Interesting discussion. Maybe Vue needs fixes applied to its package.json file, there is a long-standing PR:

https://github.com/vuejs/core/pull/4814

And maybe Vite node also needs some fixes if it's not resolving like node does 😅

Maybe a transitional solution could be a vite plugin that checks node_modules and add resolve.alias properties that follow node implementation. That way it wouldn't disrupt.

For Nuxt and Firebase, this means extending the vite config only on server;

defineNuxtConfig({
  // ...
  hooks: {
    // cannot be added in nuxt's resolve.alias
    'vite:extendConfig': (config, { isServer }) => {
      if (isServer) {
        config.resolve ??= {}
        config.resolve.alias ??= {}
        // @ts-ignore
        config.resolve.alias['firebase/firestore'] = resolve(
          nodeModules,
          'firebase/firestore/dist/index.mjs'
        )
        // @ts-ignore
        config.resolve.alias['@firebase/firestore'] = resolve(
          nodeModules,
          '@firebase/firestore/dist/index.node.mjs'
        )
      }
    },
  },
})

~~Also worth noting this only affects libraries that have different output bundles like firestore here that imports different packages for streams~~. It actually affect many other libraries as vite can end up including two copies of the same libraries, completely breaking the dev server

bluwy commented 1 year ago

I think Vue's export is mostly fine (though there could be improvements), but mostly because the Vite + Vue ecosystem had always relied on the incorrect module precedence that it causes some issues now.

I think a transitional solution make sense but perhaps implemented in plugin-vue 🤔 We could try with the fix again and make sure Histoire passes. It shouldn't be a breaking change since it is incorrect behaviour currently.

posva commented 1 year ago

I think this needs to have higher priority because it makes it impossible (or hardly debuggable) to build applications that use packages like the firebase ones (which are correctly defined based on node). As an example, a project built with Nuxt will fail in very unexpected ways, sometimes with errors, sometimes without. Adding the workaround I posted doesn't work in all scenarios. In other words, currently cannot be used to build an ssr app with Firebase 😓

vitejs / vite