Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 500 #130

Merged
merged 6 commits into from
Mar 10, 2020
Merged

Fix 500 #130

merged 6 commits into from
Mar 10, 2020

Conversation

davidqqq
Copy link
Contributor

After some investigation, we found that in Sapper updating store using .set() cause value to be undefined on server side outside reactive block but defined inside reactive block. If we switch to $ and = assignment (another acceptable syntax by SvelteDoc), the value is defined outside reactive block but undefined inside reactive block.

In Florence store value is updated using .set() and $interactionManagerContext is undefined outside reactive block. It caused a fatal error when .set() is invoked thus the 500 error.

This PR resolved this issue by moving $interactionManagerContext.select() into function that is only invoked on client side.

We suspect this is not a Florence bug but one of the Sapper's quirks. As 500 error is not consistently observed, please try to make sure we all can reproduce the error before merging.

P.S. We need to ensure all our linting is consistent and enabled.

@davidqqq davidqqq added the bug Something isn't working label Jan 23, 2020
@atepoorthuis
Copy link
Contributor

P.S. We need to ensure all our linting is consistent and enabled.

Good point. The eslint rules are in .eslintrc.json just have to make sure they're applied correctly. @davidqqq it looks like the standard rule set isn't taken into account in your linter (e.g. semi-colon everywhere). Can you check the config in your IDE?

@luucvanderzee
Copy link
Collaborator

Hi David,

How can I reproduce this issue? Is it only in the docs?
BTW, there are many other places where contexts are used in similar ways, like the Mark and Layer components. If I understand correctly, this should not be an issue in those components, right? Since the reference returned by $interactionManagerContext.select() is not assigned to a constant in the root of the component, but is accessed each time it is needed?

@johsi-k
Copy link
Contributor

johsi-k commented Feb 4, 2020

Hi Luuc,

You can reproduce the issue by navigating to site, clicking on the examples tab and refreshing the page. You should see a error like the following -

image

The error appears only on refresh and goes away if you click on the home tab and then on the examples tab.

Yes, it is not an issue in the Mark and Layer components because $interactionManagerContext.select() is not assigned to a constant in the root of the component.

Here's a toy example to better illustrate what is happening -

Case 1a: behaviour of .set() outside reactive block

input

const foo = writable('init')
console.log($foo) 

foo.set('first change')
console.log($foo) 

foo.set('second change')
console.log($foo)

client-side output

> init
> first change
> second change

server-side output

> init
> init
> init

Case 1b: behaviour of .set() inside reactive block

input

const foo = writable('init')
console.log($foo) 

foo.set('first change')

$: {
   console.log($foo)

   foo.set('second change')
   console.log($foo)
}

client-side output

> init
> first change
> second change

server-side output

> init
> first change
> first change

Case 2a: behaviour of $-prefixed variable assignment outside reactive block

input

const foo = writable('init')
console.log($foo) 

$foo = 'first change'
console.log($foo)

$foo = 'second change'
console.log($foo)

client-side output

> init
> first change
> second change

server-side output

> init
> first change
> second change

Case 2b: behaviour of $-prefixed variable assignment inside reactive block

input

const foo = writable('init')
console.log($foo) 

$foo = 'first change'

$: {
   console.log($foo)

   $foo = 'second change'
   console.log($foo)
}

client-side output

> init
> first change
> second change

server-side output

> init
> init
> second change

@atepoorthuis
Copy link
Contributor

atepoorthuis commented Feb 7, 2020

Thanks for the extensive illustration @johsi-k! It looks like this is related to a potential bug in Svelte's SSR output.

@atepoorthuis atepoorthuis merged commit dd1efe5 into master Mar 10, 2020
@atepoorthuis atepoorthuis deleted the fix-500 branch March 10, 2020 08:13
@bianchi-dy bianchi-dy mentioned this pull request Mar 18, 2020
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants