Feat/esm

[dominikg]

conversion to esm. superseeds #49

[antony]

only nitpicks

[antony]

I think pnpm added a feature recently to allow you to drop the run part. Either way I feel like this bloats the PR - makes it harder to review.

[dominikg]

that feature made me add it back, i wasn't sure if it would prefer a lint dependency over the lint script.

[lukeed]

If you use a named export you can skip the require('@sveltejs/vite-plugin-svelte').default stuff.

And since you're adding "exports" map support and type: module, this is already a breaking change, so might as well lean into it further and add the named export. The benefit of this is that it's more consistent for the user, and the type definitions will work for both ESM and CJS

[bluwy]

My 2c. I'd personally prefer if we stick with default exports in contrary to @lukeed's suggestion, to be consistent with Vite's plugin ecosystem. require('@sveltejs/vite-plugin-svelte').default is still negligible, plus most Vite configs are written in ESM instead of commonjs, so it shouldn't affect too many projects.

On that topic, I also agree with @antony that we should encourage ESM in the Vite config files.

[dominikg]

Thank you for all your comments. Greatly appreciated ❤️

I like @lukeed suggestion of a named export and had thought about it too, but ultimately went with the default setup from other vite plugins @bluwy mentioned.

Sharable typings are a valid reason to do it and maybe we can start a trend for other vite plugins to follow?
What should the export be named though? svelte is the current name used in configs and examples, any objections?

I'd love to ditch cjs completely but feel thats a bit too soon. Esp. because right now it is still needed for tests with jest.

Regarding more prominent esm examples, should the playgroud be spit into e2e and examples section?
Gotta admit its a bit messy right now.

[dominikg]

updated to named export but still included a default export with a deprecation warning:
https://github.com/sveltejs/vite-plugin-svelte/pull/54/files#diff-71fd6d8bccf1f224607bb27c94b502d32abf70103f0a2a59ad3acb5d229c443bR228

Without the default export, sveltekit breaks until updated (which includes the test in playground/kit-node)
We should coordinate a release of sveltekit that uses the named export so users can update gracefully as soon as they encounter the warning.

[bluwy]

I'm still not sure if a named svelte export is the way to go here. It could ended up like rollup-plugin-terser where no other rollup plugins actually adopts the motive.

named exports ... the type definitions will work for both ESM and CJS

Forgive me if I'm not understanding properly, but how actually does type definitions work for CJS, since types imports and exports are usually done in ESM contexts. I don't think it's a strong argument to use named exports.

And shouldn't import svelte, { Options } from "@sveltejs/vite-plugin-svelte" work too?

[dominikg]

regarding default export vs named export:

i asked about it on vite discord and while most plugins use a default export, there is eg. vite-plugin-pwa that uses a named export.
The examples on vite docs use a default export ( https://vitejs.dev/guide/api-plugin.html#transforming-custom-file-types ) as do all the official rollup plugins.

here are the links to the discussions back at terser plugin:
TrySound/rollup-plugin-terser#31 TrySound/rollup-plugin-terser#7

if it is technically more consistent i don't care much about two additional {} in an import statement, but it seems others do care and i'd like to avoid having discussions about this in the future, so ideally we can decide on a way forward here and stick to it.

so what should it be?

1. default export

• less work with sveltekit transition
• follows unwritten convention

2. named export

• improved cjs experience
• needs work to ensure smooth transition for sveltekit users

@lukeed maybe you can explain more about the benefits of the named export for cjs.

[benmccann]

Another option would be to export as named and default export, but only put the named export in the type definitions and docs and make that the preferred but not required way

[lukeed]

TypeScript only supports 1 export format in a definition file. Using a named export is the only way to expose information across ESM/CJS access. Specifically, export default excludes module.exports and this is why require('foo').default happens, even though it's not the CommonJS equivalent – it's a forced outcome of prior tooling mistakes & we're just keeping it around because we, as an ecosystem, think that's okay/how it's supposed to be. The use of .default means that the CJS source has to look like module.exports.default = or exports.default = , which is silly.

@TrySound cares about this too & that's why rollup-plugin-terser often looks like the odd one out. He's correct, though – everyone else is either pure ESM, doesn't care about CJS access, or expects Rollup users to use import & rely on Rollup to do the pre-bundle interop/conversion for them.

[bluwy]

I guess it comes down to whether we want an awkward syntax for ESM or CJS. Given a majority of our userbase is on ESM (since the default Vite starter config is in ESM), I'd prefer default exports to please the crowd.

If it all comes down to preference, perhaps a public poll could end this once and for all.

EDIT: Made a mini poll on Vite discord.

[dominikg]

removed the default export completely (as expected this caused the kit-node tests to fail).
With a coordinated release of vite-plugin-svelte and sveltekit, users of sveltekit should not run into it (unless they update vite-plugin-svelte without updating kit).