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

module field takes precedence over exports in package.json #11114

Closed
7 tasks done
posva opened this issue Nov 29, 2022 · 4 comments · Fixed by #11595
Closed
7 tasks done

module field takes precedence over exports in package.json #11114

posva opened this issue Nov 29, 2022 · 4 comments · Fixed by #11595
Labels
p4-important Violate documented behavior or significantly improves performance (priority)
Milestone

Comments

@posva
Copy link
Contributor

posva commented Nov 29, 2022

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

@bluwy
Copy link
Member

bluwy commented Dec 10, 2022

Oh this seems like the same issue I was fixing in #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 (?)

@bluwy bluwy added p3-minor-bug An edge case that only affects very specific usage (priority) and removed pending triage labels Dec 10, 2022
@posva
Copy link
Contributor Author

posva commented Dec 10, 2022

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

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
Copy link
Member

bluwy commented Dec 10, 2022

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
Copy link
Contributor Author

posva commented Dec 24, 2022

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 😓

@bluwy bluwy added p4-important Violate documented behavior or significantly improves performance (priority) and removed p3-minor-bug An edge case that only affects very specific usage (priority) labels Jan 2, 2023
@bluwy bluwy added this to the 4.1 milestone Jan 2, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Feb 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p4-important Violate documented behavior or significantly improves performance (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants