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

prefer svelte exports condition over svelte field for resolving libraries #725

Closed
dominikg opened this issue Aug 25, 2023 · 9 comments · Fixed by #747
Closed

prefer svelte exports condition over svelte field for resolving libraries #725

dominikg opened this issue Aug 25, 2023 · 9 comments · Fixed by #747
Labels
breaking breaking change - requires a major release

Comments

@dominikg
Copy link
Member

Describe the problem

We want to remove support for svelte field altogether at some point, take gradual steps to ensure existing libraries can update to not hit users with errors.

Describe the proposed solution

Start prefering the exports condition value over svelte field, add a more persistent/pressing warning for libraries that only use svelte field to switch to exports.

In a following major, drop svelte field resolve entirely. (possibly together with adding svelte 5 support)

Alternatives considered

give it more time.

Importance

would make my life easier

@Miniontoby
Copy link

Miniontoby commented May 29, 2024

Hi,

I would like to report that you guys should edit the IF statement used to show a warning when a package is still having the svelte field in the root of the package.json in addition to the exports condition.

On the FAQ it is said that:

For backwards compatibility, you can keep the svelte field in addition to the exports condition. But make sure that both always resolve to the same files.

However there is a package svelte-dnd-action and using that package, we get the warning:

The following packages use a svelte resolve configuration in package.json that has conflicting results and is going to cause problems future.

However it has the exports condition in combination with the svelte field:
https://github.com/isaacHagoel/svelte-dnd-action/blob/aa5e28d7a9da4210e671409e45ab9d17cdab21b4/package.json#L29

P.s. yes the value's aren't exactly the same. One of them starts with ./ and the other has the ./ omitted, but if I edit that to be exactly the same, then the warning still shows up!

@dominikg
Copy link
Member Author

dominikg commented May 29, 2024

@Miniontoby please file an issue including a link to a repository with a reproduction.

in general i think the exports map of that package could be changed a bit to put "svelte" into the "import" condition, otherwise you might end up resolving it via exports.import.default instead of exports.svelte. (unfortunately in exports maps order is relevant and first matching entry "wins").

so

    "exports": {
        ".": {
            "import": {
                "types": "./dist/index.d.ts",
                "default": "./dist/index.mjs"
            },
            "require": {
                "types": "./dist/index.d.ts",
                "default": "./dist/index.js"
            },
            "svelte": "./src/index.js"
        }
    },

should be

    "exports": {
        ".": {
            "import": {
                "types": "./dist/index.d.ts",
                "svelte": "./src/index.js",
                "default": "./dist/index.mjs"
            },
            "require": {
                "types": "./dist/index.d.ts",
                "default": "./dist/index.js"
            },
        }
    },

note that removing keys from an exports map is a breaking change.

vite-plugin-svelte does not validate the right hand side of the exports condition. it only checks if there is a "svelte" key somewhere in the json structure of the exports map. code

please also check your package type field, it looks like .js might currently be mistaken as commonjs.

https://publint.dev/svelte-dnd-action

@Miniontoby
Copy link

I might file an issue later this week, but yeah since this is the issue that introduced it, I thought it would fit here....

But yeah the code you linked to is used for showing the packagesWithoutSvelteExportsCondition message, however that is NOT what I get, it is the The following packages use a svelte resolve configuration in package.json that has conflicting results and is going to cause problems future. message that I get/see.

The message that I see doesn't seem to appear anywhere here in this repo tho... Nor in the whole sveltejs org (see this search)
So idk if that might be because I am still using svelte v4 since if I update, then my other packages won't install...

And btw the svelte-dnd-action is NOT my repo/package.

@dominikg
Copy link
Member Author

oh, sorry for not looking at that message more closely.

This particular warning had been added in #510 and then removed again in #747 it is only present in vite-plugin-svelte@2. So the easiest way for you to avoid it is to upgrade to vite-plugin-svelte@3.

@Miniontoby
Copy link

Miniontoby commented May 29, 2024

Uhmmmm.... I don't even have vite-plugin-svelte installed at all. Nor does the svelte-dnd-action package has...

Oh wait it is in the @sveltejs/kit

But @sveltejs/kit doesn't yet use vite-plugin-svelte@3.

@dominikg
Copy link
Member Author

@sveltejs/kit@1 had a direct dependency on it, yes. you can update to kit@2 , that has a peerDependency on vite-plugin-svelte@3.

@Miniontoby
Copy link

Miniontoby commented May 29, 2024

Hmm the update didn't show up using npm-upgrade, seems like I was still using older packages with like next versions... and specifically the eslint-plugin-svelte3 package... also mostly when I tried updating packages, I got ERESOLVE...
But I seem to finally have fixed that problem.

However, I am now updated and I do now get that error that you were talking about:

21:14:21 [vite-plugin-svelte] WARNING: The following packages have a svelte field in their package.json but no exports condition for svelte.

svelte-tauri-filedrop@1.0.0

Please see https://github.com/sveltejs/vite-plugin-svelte/blob/main/docs/faq.md#missing-exports-condition for details.

Update: Oh wait, that is a completely different package!

@dominikg
Copy link
Member Author

dominikg commented May 29, 2024

that one is legit, the package doesn't have one. Please tell the author. For further help please check our discord on https://svelte.dev/chat channel #help-svelte-and-kit.

@Miniontoby
Copy link

Wait, this is strange, cause the original source of this other svelte-tauri-filedrop@1.0.0 plugin doesn't have the svelte field at all... but still my node_modules/svelte-tauri-filedrop/package.json does have that...

Well, I removed the fields that were added somehow and it is fixed. Thanks for you help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking breaking change - requires a major release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants