Skip to content

feat: resolve svelte in exports. prefer exports to svelte field #459

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

Closed
wants to merge 7 commits into from

Conversation

benmccann
Copy link
Member

Closes #421

We shouldn't have our own resolving code path as it will fail to account for things like Vite's pre-bundling. If we really wanted to we could try to expose something from Vite and use if here. But I think that is not the best course of action as we're better off simply treating the svelte field as a boolean

@dominikg
Copy link
Member

dominikg commented Oct 8, 2022

This requires a changeset and it is a breaking change as a lot of existing libraries use it.

Are you sure we don't need a replacement for custom resolving of .svelte files?

How would other bundler plugins (rollup, webpack,?) handle this?

Feels very large and important, so additional tests, documentation and communication to library authors are needed here. Ideally we would find a way to have legacy support for svelte field for a while and drop it only with the next svelte major.

@benmccann
Copy link
Member Author

benmccann commented Oct 8, 2022

The main place this is documented is rollup-plugin-svelte readme. I had been thinking that the svelte field and entry point were always the same as that's the only thing I've ever seen, but it appears that we do recommend a case where they might differ:

if you're publishing a component to npm, you should ship the uncompiled source (together with the compiled distributable, for people who aren't using Svelte elsewhere in their app)

I think what we could do instead is have vite-plugin-svelte use the config hook to provide svelte via resolve.conditions. I'm not sure if that would pick up the svelte field. If it does then it'd be backwards compatible. If it doesn't and only works with exports then we can leave the current code path in place, but print a warning that the svelte field should be moved to exports

@bluwy
Copy link
Member

bluwy commented Oct 9, 2022

If it doesn't and only works with exports then we can leave the current code path in place, but print a warning that the svelte field should be moved to exports

I tried this before and it only works with exports. I agree with dominik though that we should have legacy support and a deprecation warning if we want to move away from it.

I also think we should remove pkg.svelte altogether if we're already planning a breaking change. We can check exports.**.svelte instead for the same behaviour. Unless we don't want to go with exports.**.svelte, and prefer a direct exports.**.import instead 🤔

@dominikg
Copy link
Member

dominikg commented Oct 9, 2022

Do we need an RFC for that or does it "just work"? last time i looked at exports conditions it wasn't that great and iirc it still requires a flag to be used in node itself.

How would it work for hybrid libraries that want to offer import {serverside} from 'some-lib' as well as import {ClientComponent} from 'some-lib' ?

@dominikg
Copy link
Member

dominikg commented Oct 9, 2022

Just in case this wasn't clear: I completely agree with removing the "svelte" field, but really want to avoid breaking existing libs in the process.

@benmccann
Copy link
Member Author

How would it work for hybrid libraries that want to offer import {serverside} from 'some-lib' as well as import {ClientComponent} from 'some-lib' ?

I might be misunderstanding, but that looks like two different named exports from an index.js file, so you would just reference them exactly like that. I'm not exactly sure where the difficulty lies?

a package could expose uncompiled files alongside a standalone dist file

Are you talking about something like this usecase that Rich mentioned? In that case, vite-plugin-svelte or SvelteKit could provide the svelte resolve option to get the uncompiled version, but someone not providing the svelte resolve option would the the compiled version.

@benmccann benmccann changed the title fix: don't resolve svelte field fix: let Vite resolve svelte field Oct 14, 2022
@benmccann benmccann changed the title fix: let Vite resolve svelte field feat: resolve svelte in exports. prefer exports to svelte field Oct 14, 2022
@benmccann benmccann force-pushed the no-svelte-field branch 2 times, most recently from b7c07be to cc769df Compare October 14, 2022 22:25
@benmccann
Copy link
Member Author

Closing for now. We should add a warning period first: #501

@benmccann benmccann closed this Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resolve @svelteuidev/core to prebundled version
3 participants