Svelte: CommonJS output is non-idiomatic

[TehShrike]

Right now (Svelte 3.0.1), when you call compile with format: 'cjs', the final line of the output code is

exports.default = Test;

which seems like a bug – usually you only see default in CommonJS-land when you're dealing with ESM modules that were bundled to CJS. You wouldn't expect to see default in a vanilla CJS module.

It seems like the export should just be

exports = Test;

[Rich-Harris]

I think this causes more problems than it solves in the long run, no? It changes the component's semantics (any named exports becomes static properties of the component itself, unlike in ESM) and I imagine it would break import Thing from './cjs/Thing.svelte'. (Though maybe we need to add exports.__esModule = true for that)

[TehShrike]

It changes the component's semantics

Can a component definition ever actually export anything besides the constructor function? I thought all other exports were interpreted as props, instead of being something that it made sense to actually import/require somewhere else.

import Thing from './cjs/Thing.svelte'

This behavior is undefined as far as I'm concerned – different bundlers have different behavior here.

Realistically, since Svelte is ESM-first, I think the only folks who will be compiling to CommonJS will be ones targeting node, or browserify.

const SomeComponent = require('./SomeComponent.html').default looks really awkward, I'd probably add a nasty hack to sveltify rather than force that on browserify users

[Rich-Harris]

Can a component definition ever actually export anything besides the constructor function?

yep: https://svelte.dev/tutorial/module-exports

[TehShrike]

derp, forgot about context="module"

[TehShrike]

I'm still leaning towards the idiomatic CJS thing being

AudioPlayer.stopAll = stopAll
module.exports = AudioPlayer

[EmilTholin]

This might be worth exploring as well. Babel did this back at version 5, but it got removed for spec compatibility.

[steve-taylor]

CommonJS output also breaks import foo from 'bar'; in a Svelte component. foo ends up being undefined.

[cam-stitt]

An issue that may potentially be caused by the current CJS format. When using Svelte 3 with Jest, I'm having to access default on the imported component:

import Component from './Component.html';

new Component.default({});

It is a strange quirk to have to remember to add the .default to all test cases.

[MaxMilton]

@cam-stitt for the time being, in your jest transformer you can inject the es module interop code to prevent needing to call .default. See example: https://github.com/WeAreGenki/minna-ui/blob/dev/utils/jest-config/src/svelte-transform.ts#L23

I admit though it's a bit awkward for svelte components to use exports.default _without_ the es module interop too.

[TehShrike]

The current sveltify workaround is to replace exports.default = with module.exports =

https://github.com/TehShrike/sveltify/blob/master/index.js#L47

[TehShrike]

IT'S BAD BUT I'LL FIX IT LATER

[cam-stitt]

Thanks for the fix @MaxMilton it is a handy workaround. It seems though, that some of the dependent imports in my components also fail when using CJS. Has anyone else had that issue? One of the difficulties is working out if it's caused by Jest or Svelte :thinking:. I think it's the svelte changes, as the code worked fine for svelte 2.

[MaxMilton]

@cam-stitt my guess is it may be related to your jest config. Do you have any example repo?

[cam-stitt]

I managed to resolve the issue by adding the interop to my jest transform
for svg assets. I didn't need this for svelte 2, but seems to resolve the
issue now.

On Thu, 9 May 2019, 07:35 Max Milton, [e-mail] wrote:

@cam-stitt https://github.com/cam-stitt my guess is it may be related
to your jest config. Do you have any example repo?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/sveltejs/svelte/issues/2562#issuecomment-490659478,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABNSE7MWR7M3IYJWXDC4JTPUNBQFANCNFSM4HINOC2Q
.

[Conduitry]

I feel like it's probably been long enough that, whether or not this was the right decision before, changing it now would definitely be a breaking change. Thoughts?

[TehShrike]

I think this could still be fixed in a backwards-compatible way?

module.exports = Object.assign(Test, moduleExports, {
    default: Test
})

[TrySound]

Oh, it's happening again. Don't you think it's much easier to avoid default exports completely? They were added only to interop with commonjs and does not really serve this purpose but make it even more complex. Considering module as a namespace works well for many language.

Just an observation. I see a few new issues related to this every month.