Skip to content

Input type range bound value 0 is omitted from ssr #4551

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

Closed
jonniek opened this issue Mar 13, 2020 · 2 comments · Fixed by #6458
Closed

Input type range bound value 0 is omitted from ssr #4551

jonniek opened this issue Mar 13, 2020 · 2 comments · Fixed by #6458
Labels

Comments

@jonniek
Copy link
Contributor

jonniek commented Mar 13, 2020

Describe the bug
The range element with a bound value 0 is omitted from server side rendering. This leads to the range input jumping(from default location) after client side js takes over. This issue seems to stem from 0 being a falsy value in Javascript.

To Reproduce
To reproduce you can use this repository by Rich https://github.com/Rich-Harris/svelte-d3-arc-demo
Updating the dependencies to latest versions does not solve the issue. If the client side JS takes over too fast you can disable JS or throttle the network to observe the issue.

The only change you need to do before building is in this file /src/Viz.svelte

-       let angle = Math.PI * 2;
+       let angle = 0;

After building you can observe the following in /build/Viz-ssr.js

let angle = 0;
...
<input ... ${(v => v ? ("value" + (v === true ? "" : "=" + JSON.stringify(v))) : "")(angle)}>`;

The resulting index.html does not contain a value for the range input.

<input type="range" min="0" max="6.283185307179586" step="0.01" class="svelte-1qzw59f" >

Expected behavior
Input type range value 0 is included to the html.

Information about your Svelte project:

  • Firefox nightly

  • Windows 10

  • Svelte version 3.19.2

  • Rollup

Severity
Low severity since this can be worked around by offsetting the range to not include 0. But it's a bit annoying especially if you have a bad approach for reverting the offset.

My current workaround:

let daysAgoWithOffset = 1
$: daysAgo = daysAgoWithOffset - 1

$: recentEvents = events.filter(
    ...
    const daysAgoMS = daysAgo * oneDayInMS
    ...


<input bind:value={daysAgoWithOffset} type="range" min={1} max={32} step={1}>

edit: seems like let angle = "0"; might be a more simple workaround if you don't need math or number methods

Additional context
This issue also happens for for any binding of integers such as with number input. However, it is not quite as visually displeasing as only the value changes from empty to 0, rather than moving around the window.

@jonniek
Copy link
Contributor Author

jonniek commented Mar 13, 2020

I guess the issue is located here

} else {
const snippet = expression.node;
renderer.add_expression(x`@add_attribute("${name}", ${snippet}, 1)`);

export function add_attribute(name, value, boolean) {
if (value == null || (boolean && !value)) return '';
return ` ${name}${value === true ? '' : `=${typeof value === 'string' ? JSON.stringify(escape(value)) : `"${value}"`}`}`;

Would it be enough to check boolean && !value && value !== 0)? I guess if some consumers are using 0 1 boolean logic then this is not an option. It just seems like a lot more work to do some kind of whitelist in the Element.ts

My environment is not passing all tests so I'm currently having a bit of a hard time to test solutions.

@Conduitry
Copy link
Member

This should be fixed now in 3.39.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants