fix set store value ssr

[tanhauhau]

Closes #3375

Have to instrument setting store variable, store to sync up the dollar-prefixed variable, $store to simulate the same behavior in DOM.

An alternative would be to subscribe to store at the early of the component, and make sure to unsubscribe them right before rendering them.

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with npm test and lint the project with npm run lint

[Conduitry]

Should the description here say 'fixes #3375 and closes #4195'?

[Conduitry]

Does this seem like a good base to start from when fixing #3582 / #3636? And/or does it make sense to also fix those in this PR?

[tanhauhau]

wow, the 2 issues seems hard, especially, it is modifying the store directly via store.set() for example.

1 alternative i can think of it is to subscribe to the store early on, which the listener can keep updating $-prefixed variable, and unsubscribe right before returning the rendered result.

what do you think?

[Conduitry]

I think having a subscription to the store throughout the SSR'd component that gets unsubscribed at the end makes sense. It does sound like that would help with those other issues - and if we can align what the SSR component does more closely with what the browser component does, that sounds like a good idea.

[Conduitry]

I saw you mention this PR in #5452 ... Is this PR fixing an observable bug? #3375 just sounded like some suboptimal code getting generated. How does what this is fixing compare to #3582 and #3636?

[tanhauhau]

probably im gonna change the implementation of this PR / start a new PR that does "having a subscription to the store throughout the SSR'd component that gets unsubscribed at the end"

that would theoratically solve most of the ssr + svelte store issues.

#3375's suboptimal code is because, svelte store value can be changed throughout the main code, via store.set() or store.update(). so a safe way to make sure that the $store values is up to date, we read from the store once last time before returning.

again, if we change the store mechanism in ssr, having a subscription, we can probably remove that part of the code too.

[Conduitry]

I'm marking this as draft. If you'd prefer to rework SSR store subscription in a separate PR, this can be closed.

[benmccann]

can you rebase against master and make sure npm run lint passes? we just updated our eslint config to allow only single quotes

[arackaf]

@Conduitry @antony

Other than the linting problems described above, is there anything else preventing this PR from being merged? I'd be happy to rebase and fix the lint errors if it'll help get this merged.

[tanhauhau]

@arackaf there's some edge cases that this MR didn't manage to fix as mentioned in #5419 (comment), I'll probably relook again soon

[arackaf]

@tanhauhau if you're still planning on taking a closer look to solve those other edge cases then cool, I'll leave you to it.

But if you decide to move on, I'd be more than happy to help get this incomplete solution in, since, iiuc, it does solve some problems.

[cortopy]

any change this could be merged?

My use case would be fixed with this and I suspect many other people would benefit from this early implementation

If there are any edge cases not covered, could they not be treated as an improvement to work on in a different PR?