sveltejs / vite-plugin-svelte

Svelte plugin for http://vitejs.dev/
MIT License
844 stars 103 forks source link

Getting error with svelte-fa library after 3.0.1 update #813

Closed benoitf closed 9 months ago

benoitf commented 10 months ago

Describe the bug

I'm getting the error No matching export in "node_modules/svelte/src/runtime/internal/index.js" for import "get_slot_context"

node_modules/svelte-fa/dist/svelte-fa.mjs:1:140:
  1 │ import { SvelteComponent, init, safe_not_equal, element, append, svg_element, attr, toggle_class, insert, detach, empty, noop, create_slot, get_slot_context, get_slot_changes, transition_in, transition_out } from 'svelte/internal';
    ╵

image

it's related to https://github.com/sveltejs/vite-plugin-svelte/pull/809

Reproduction URL

https://github.com/benoitf/svelte-issue-app

Reproduction

  1. clone the repository https://github.com/benoitf/svelte-issue-app (it's a repo created with npm create vite@latest svelte-issue-app -- --template svelte
  2. run yarn
  3. run yarn dev

now, edit package.json and specify 3.0.0 for vite-plugin-svelte, ran again yarn, here it works

Logs

No response

System Info

System:
    OS: macOS 14.2
    CPU: (12) arm64 Apple M2 Max
    Memory: 2.85 GB / 96.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.16.0 - ~/.nvm/versions/node/v18.16.0/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 9.5.1 - ~/.nvm/versions/node/v18.16.0/bin/npm
  Browsers:
    Chrome: 119.0.6045.159
    Safari: 17.0
  npmPackages:
    @sveltejs/vite-plugin-svelte: 3.0.1 => 3.0.1
    svelte: ^4.2.3 => 4.2.7
    vite: ^5.0.0 => 5.0.2
dominikg commented 10 months ago

soo this one is a doozy.

1) The package.json of svelte-fa is a mess. It exports precompiled svelte components (with an older version of svelte 3) from the browser and module fields. This is a bad practice. It should switch to svelte-package and only distribute the uncompiled .svelte file. If it wants to distribute a built version, it should build it as a custom element instead. (but again don't recommend that).

2) This messy config worked in vite-plugin-svelte 2 because the svelte field was always resolved first by our own custom logic, sidestepping the mess in browser/module. In version 3, we changed that to resolve with vite and prefer exports condition over the svelte field.

3) You might ask why this did not break in 3.0.0 then, and the change in 3.0.1 is the reason for it. We initially missed the new browser field in vite's own mainFields list, which we have to copy to add svelte at the start of it. After we did this, svelte-fas package.json triggered this special case in vite resolve: https://github.com/vitejs/vite/blob/096dbdccd9dd8030d5ddc152e0390828e7a98a3f/packages/vite/src/node/plugins/resolve.ts#L986 which says that if both browser and module fields exist and browser is not esm, then use module.

It does that regardless of position of browser in the mainFields list, so this is going to have to be fixed in vite (i'll work on that afterwards).

The short term workaround for you could be to go back to 3.0.0 or import from svelte-fr/src/fa.svelte directly

credits to @bluwy for finding the vite code ref

benoitf commented 10 months ago

thanks for the explanation @dominikg (and @bluwy then as well)

For now yes I've fixed the plugin to 3.0.0 (and yes usually you don't expect big breakage on .z bugfix release)

I can probably report the issue on the svelte-fa repository as well. It can be better to have a new release version there as well.

dominikg commented 10 months ago

please do, distributing compiled svelte code is not working well. best case users have 2 different svelte versions at runtime that just waste size, then it could just crash with incompatibilities like here, or worst case they'll end up with larger output that has subtle bugs due disagreements betweeen them.

My recommendation would be to swtich to svelte-package , add "type": "module", add an "exports" map , add publint to verify their package.json is ok. And last but not least maybe don't do mixed exports (default and named) from the index file. Thats not great.

dominikg commented 9 months ago

this has been fixed in vite5, svelte-fa should still clean up their package.json