sveltejs/rollup-plugin-svelte

Fails building when `export const` is used (without the initializer)

emirotin opened this issue · 8 comments

I have an exported property that's unused.

When it's exported as export let params it prints a warning:
image

When I switch it to export const it fails to build completely:
image

Minimal repro: https://github.com/emirotin/svelte-parse-error, based on the official template and only adds a single line to it.
Run npm run build and you'll see the warning.
Change https://github.com/emirotin/svelte-parse-error/blob/master/src/App.svelte#L3 to const and you'll get the error.

It works when you add an initializer to the prop export const unused = 1 though.

I think this makes sense but at the very minimum, the plugin recommendation provides misleading advice.

Your first error comes from the fact that the property is unused. The warning is valid. If it's not used, remove it. If it is, then assign it where it is required.

The second error is also valid. You can't create a constant without a value, because it is, as defined by const, constant. It needs an initial value, as its value cannot be reassigned later, that's a javascript language feature.

The error isn't misleading, since it's a valid suggestion - it would be out of scope to try to teach javascript language fundamentals in an error message from Svelte.

While the errors are valid (which I have mentioned) the recommendation for the first one is misleading IMO.
Given that Svelte is not exactly JS I expected a piece of non-standard JS could be legal here so I actually tried what was literally recommended in the warning and got an error.

At the very minimum, I'd expect the first warning to recommend removing it (which it does not) and also provide the valid example syntax like export const params = someValue.

The second error is also kinda misleading because it suggests the reason can be because of the non-JS syntax that may be valid TS. While I understand it's generic I find it a not-so-great DX.

Given that Svelte is not exactly JS I expected a piece of non-standard JS could be legal here

It wouldn't make sense for svelte to change the semantic meaning of the const keyword, to do something completely contrary, no.

At the very minimum, I'd expect the first warning to recommend removing it (which it does not) and also provide the valid example syntax like export const params = someValue.

I think the reason we don't say remove it is because it could be assigned to a value and read somewhere else, in which case it should be a const, since the only bit we can actually know is that it isn't used in this component. It certainly isn't a good idea to try to suggest a value for a variable that we don't even know the purpose of.

The second error is also kinda misleading because it suggests the reason can be because of the non-JS syntax that may be valid TS.

Not sure I understand what you're saying about TS here, but the message simply suggests that the definition keyword is the wrong one for the use-case, and I think including any further detail than simply "this variable isn't modified in this component, so could be defined as a const instead" is basically as involved as it would be sensible to get.

Okay, you're in your right to not take any action here, obviously.

But I'm an experienced senior JS developer, and if I was baffled by the DX here maybe it could be improved.

UPD: Why in the first place would I add this unused export. If I do not there's a warning from the router component I use when this property is absent from the component.
image
I'm a bit of a purist so I wanted to avoid getting this warning. This project is for my students and I don't want them confused. Now, there's no way for me to solve it:

  • missing prop — a client-side warning from the component
  • unused prop with let — a build-time warning from the compiler
  • unused prop with const and without the initializer — a build-time error
  • unused prop with const and with the initializer — the component is not impressed, the same warning

Sorry for the additional comment, I remembered there was a reason for this in the first place but I forgot how to repro it.

If you export a prop within a component you should either reference it within that component, turn it into a const if it is used by external parties only (or move it there), or remove it entirely.

If your router demands that you declare properties even if you don't use them, then the problem might exist with your router.

Perhaps a REPL would demonstrate the specific issue better.

Yes exactly, I'm using an otherwise OK router library that is willing to pass a prop to my component.

But in this specific component, I don't actually need this prop. Because it's a no-params route, an entry point to my client-side app. (I need this component though because it's a catch-all component that manages the nested routes.)

I don't know how REPL is useful here (though I have created it https://svelte.dev/repl/4deaa36574b34b5f95c52a8f6eb637be?version=3.31.0) because I don't see how I can access the console there.

The console can be found by clicking the box marked console in the bottom right.

Looking at it there is a good way for me to understand the problem, however the problem is one for which svelte-spa-router is responsible for, and should probably be fixed there.

Improving the warning message doesn't make the warning go away, it just makes it more detailed. In this instance however, I'm just not convinced of the value of making it more detailed. It's unlikely telling people how to use the const keyword is going to result in desired behaviour.