Describe the bug
Since #5079, valid code is causing the following error:
Stores must be declared at the top level of the component (this may change in a future version of Svelte)
To Reproduce
https://svelte.dev/repl/8240f1807eda4d429018826229910e48?version=3.24.0
It's coming from this code:
<script>
import { getChartContext } from './Chart.svelte';
import { get_ticks } from '../utils/ticks.mjs';
// ...
const { x1, y1, x2, y2, x, y } = getChartContext();
// ...
$: style = orientation === HORIZONTAL
? (y, i) => `width: 100%; height: 0; top: ${$y(y, i)}%`
: (x, i) => `width: 0; height: 100%; left: ${$x(x, i)}%`;
</script>
The y that is part of the chart context is a store; the y inside the closure is a number.
Expected behavior
No error
Severity
Moderate. Requires either downgrading Svelte in our project, or working around the bug in Pancake
Ah — looks like someone caught this a few days ago https://github.com/Rich-Harris/pancake/issues/19
Yeah I tried to solicit input on #5079. It is technically a breaking change, but I'd argue that the code that is now disallowed was confusing. I'd kind of push for changing components that were affected by this, rather than rolling back part of this check. Having a scope where foo and $foo refer to different variables seems quite confusing.
Yeah, I guess in this situation we get to decide whether the code is 'valid' or not. For the time being at least I've made the change in pancake. I guess I wouldn't be put out if we closed this accordingly; at least there's an issue to land on if someone searches for the text of the error now.
Something else I had mentioned on that other PR - if we ever plan to allow subscribing to stores that are not defined at the top-level, that would have itself technically been a breaking change vs. the old behavior. (Because using $foo in a scope where foo was shadowing a top-level foo would suddenly mean something different.) Having a nice long intervening period where it throws a compile time error seems a lot safer.
Most helpful comment
Something else I had mentioned on that other PR - if we ever plan to allow subscribing to stores that are not defined at the top-level, that would have itself technically been a breaking change vs. the old behavior. (Because using
$fooin a scope wherefoowas shadowing a top-levelfoowould suddenly mean something different.) Having a nice long intervening period where it throws a compile time error seems a lot safer.