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

warn if removing svelte resolve override would break a package #501

Closed
benmccann opened this issue Nov 16, 2022 · 2 comments · Fixed by #510
Closed

warn if removing svelte resolve override would break a package #501

benmccann opened this issue Nov 16, 2022 · 2 comments · Fixed by #510
Labels
enhancement New feature or request

Comments

@benmccann
Copy link
Member

benmccann commented Nov 16, 2022

Describe the problem

Right now the svelte field is resolved first. We should eventually remove this and defer to Vite which will resolve exports first

Describe the proposed solution

Add dependency on @alloc/resolve.exports to resolve the exports. If exports exist and resolve to different place than svelte field then warn.

Alternatives considered

Discussed on Discord the the maintainers group and I think this is what we arrived at. I won't bother repeating the discussion

Importance

nice to have

@benmccann benmccann added enhancement New feature or request triage Awaiting triage by a project member labels Nov 16, 2022
@dominikg
Copy link
Member

dominikg commented Nov 17, 2022

love the idea of testing if resolve returns a different result with or without the svelte field.

We would have to issue a warning too for the case if removing the svelte field would fail resolving entirely if there is no exports map or other entries. Right now this package.json is legal for svelte libraries

{
  "name": "some-lib",
  "svelte": "index.js",
  "type": "module"
}

There are also hybrid libraries that export node code as well as svelte files and some might leverage different "main" and "svelte" indexes for this.

not sure we need the external lib for this, can't we just leverage vite's this.resolve or something ?

@benmccann
Copy link
Member Author

@dummdidumm suggested continuing to support the svelte field, which seems reasonable to me. I think the thing we really need to change is that it has precedence over exports. In that case we wouldn't need any warnings for the example file above.

I didn't realize Vite exported this.resolve. Using that instead of an external library sounds good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants