window.scrollTo and SSR

[Wolfr]

I have some line of code that resets the window scroll position on every page change:

<script>
    import { afterUpdate } from 'svelte';

    import Header from '../UI/Header.svelte';
    import Footer from '../UI/Footer.svelte';

    afterUpdate(() => {
        window.scrollTo(0, 0);
    });

</script>

<Header />

<slot></slot>

If I run a build with npm run build, I get an error.

Error: Not implemented: window.scrollTo
    at module.exports (/Users/username/Sites/mono-svelte/node_modules/jsdom/lib/jsdom/browser/not-implemented.js:9:17)
    at /Users/username/Sites/mono-svelte/node_modules/jsdom/lib/jsdom/browser/Window.js:714:7
    at eval (<anonymous>) undefined

Now... how do I deal with this? Any suggestions? If I remove the code, I will not have the scroll reset.

What's the smart way to go about the rest of the implementation.

I read this article yesterday which seemed interesting enough (but it's for React).

[jakobrosenberg]

Routify should scroll to the top automatically on page change.

In general you could check if the function exists before running it. (Some functions don't exist or aren't polyfilled in node)

[Wolfr]

OK, I'll remove the code and test. It was in place because I was using an older version where it wasn't the case. Thx for the advice!

[jakobrosenberg]

It was also broken in a newer version. 😂

If you get a similar error in the future, you can often do something like this:

window.someFunction && window.someFunction(0, 0)

or

if (window.someFunction) window.someFunction(0, 0)

You should of course consider if the app works without the function when rendered on the server. In this case scrollTo has no effect as SSR renders each page from scratch and thus has no stale scroll position.

[rixo]

Shouldn't we include this kind of polyfills, for which we are 100% sure than noop is the right solution, by default? If we already have some polyfills, that means we already live in a mixed world; I see no value in letting people run into issues we can safely prevent. (There could be some pedagogical value if they had to understand for it to work at all, but here it's more a question of more or less automatic support -- I'd favor more.)

[Wolfr]

I'm all for good defaults. This one specifically should be part of the framework imo. I'll include the function for now in my project.

[jakobrosenberg]

I'm for polyfills as long as either the behavior is predictable or we print warnings from our SSR function.

Ie. something like

Rendered src/pages/about.svelte with empty polyfill for window.scrollTo

That way pages will still be rendered and if you experience odd behavior on server rendered pages, you'll have an idea why.

[Wolfr]

Both above solutions still lead to the same error. I tried something else but to no avail.

if (typeof window !== 'undefined') {
    window.scrollTo(0, 0);
}

Then I went on a google search and found jsdom/jsdom#1537 ; there's a solution in there - current code in full is

<script>
    import { afterUpdate } from 'svelte';

    import Header from '../UI/Header.svelte';

    afterUpdate(() => {
        if (!navigator.userAgent.includes("jsdom")) {
            window.scrollTo(0, 0);
        }
    });

</script>

<Header />

<slot></slot>

Perhaps this can be implemented in Routify itself.

[jakobrosenberg]

Checking if window exists won't work as it polyfilled by JSDOM. You need to check for the specific function instead.

if (window.scrollTo) window.scrollTo(0, 0)

[Wolfr]

I tried your solution before trying something else. But it doesn't work... try and execute the code you posted in a build and you will see you get the same error as in the opening post.

[rixo]

Yes, it exists. The exception is raised from the entrails of jsdom, so it has a stub scrollTo that throws this error... It also mocks window, so you have to detect jsdom specifically, as indicated in the issue you've linked:

const isJsDom = navigator.userAgent.includes("Node.js") || navigator.userAgent.includes("jsdom")
if (!isJsDom) {
  window.scrollTo(0, 0)
}

domenic implied in 2016 that navigator.userAgent.includes("jsdom") alone should suffice by now, so it would be great if you can try just that and confirm.

This code is ugly, so that's all the more reasons to include as much such polyfills as we can to save our users from that.

I'm for polyfills as long as either the behavior is predictable or we print warnings from our SSR function.

I'm very confident this one does not deserve a warning. There's no way scrolling in SSR can influence the behaviour on the client-side, does it? Remember that it's just the HTML being generated. The JS will always be executed again in the client.

Or maybe we put warnings for everything (because next cases we encounter might be less clear cut), but we make them optin by default (that is we display them only with a specific cli flag is passed -- e.g. --ssr-warnings). The use case would be to turn this on when the user encounters fishy behaviour and enter "full diagnostic mode". Crowding the console with warnings just makes them all ineffective...

Also, we have to find a way for the source maps to work for the eval'd code because those orphan errors are horrible horrible DX. Maybe setting inline source maps with sourcemap: true sourcemap: 'inline' (edit: just checked the docs) in the config for SSR build would be enough to do the trick.

This little issues yields more than it first appeared it would...

[Wolfr]

I don't even like having CLI flags for warnings, the code needs to be kept simple and understandable. In my understanding user expectation is that on a new page, scroll position in reset. In an SPA, this needs to be built into the router. We can't expect every user of the router to figure this out for themselves. The same counts for defocussing the page (#87).

It's exactly this sort of thing where you need to make a clear decision that it's built-in and it just works that way, otherwise you will implement a compromise, and it leads to having to add and maintain more documentation and CLI flags, and then your project goes in the wrong direction.

I've been involved with Routify for +-2 months and only the past days I actively looked at the CLI flags... just to give you an idea of my perspective.

[rixo]

The "warnings" we're talking about here are to inform the user about a layer of behaviour that has been added by Routify and might be the cause of some problems for them, namely browsers polyfills in jsdom. Sadly, they might be needed to troubleshot in some cases. If @jakobrosenberg thinks we need warnings and is hot on implementing them, I'm all for it. Just not display them by default, 'cause they're noise and probably unneeded ~99% of the time.

Now, adding or not adding scrollTo(0, 0) or defocusing is another subject entirely. I agree that being accessible by default is the right thing to do, should be a high priority, and we should expand the effort to do it correctly. In part, I think this is something that is expected from the new generation of tooling we are building because the previous generations have been lacking, and there's no point in doing new if it's not for doing better.

That being said, I strongly believe that there should be an escape hatch for all of these added default behaviours. That is, there should be well documented options to disable them.

For one thing, it has a strong pedagogical value, even in the case the user doesn't want to disable them, to see that they exist, that Routify does that, and to read about why it does that. People might not actively Google for specific accessibility issues (you have to know about them, I would have never thought about the focus thing), but many will surely read the docs of the tool they're trying to use.

And also, not everybody is using a router to do site-like applications. Some may be doing games, or shiny demos that are not intended to be used by anybody else than themselves, or whatever, and may actually have to disable them to make things work properly. Trapping them into a technically easy to disable extra behaviour for moral reasons makes no sense.

The principled and clear decision is expressed enough in the fact that we make it a default. And I think this is the stronger we can respectfully do. "it just works that way" might work well (for some people) for a complete product (say a car... or an OS), but we're a tool. You want your tools to be versatile. You don't want your power drill to come with a single bit you can't change...

We don't want to be authoritarian about our defaults for no reasons. Keep in mind that a lot of people come to Routify precisely because Sapper has too much unescapable defaults in its current form. We want Routify to be complete and soundly defaulted, BUT we also want it to be modular, and customizable, in order for it to adjust to everyone's needs. We want to be inclusive. We want freedom. And yes, that means we also want to support people who will disable accessibility for an anecdotal UI win, even if we personally don't want them to do that. It's like free speech: that's the point.

(Sorry, got carried away... Happens with me :-x)

[Wolfr]

Flags and escape hatches for behavior can be good, I am not against them, just warning about the maintenance cost. And worried because we are a small team doing this mostly in free time. Now also, I agree that it's too authoritarian to say: “that's the way it is. Sorry. We can't help.” That stance in OSS will definitely not increase adoption.

In general education is good as well (I am learning a lot from Gatsby's research on this, their blog posts, public discussion). But we have to separate author land from user land.

It's my opinion that optimizing for situations that don't exist leads to convoluted software. I am well aware that Svelte is used for game-like UI, but in that case, you are likely to stay on the same "page" (i.e. the game has persistent UI). But I don't like this hypothetical stuff. I am presenting you with real-life issues trying to ship a good website with Svelte. We also have an alternate development track we are working on where we revert back to Wordpress.

I would like to use Svelte + Routify for our websites, but we can't ship an inaccessible, non-indexable website.

Great to have our first strategy discussion 😅

[rixo]

Of course.

As I read you, we're entirely on the same line... I keep saying accessibility should be high priority, and I do think we should optimize the defaults for the main use case -- that is, site-like applications.

I don't advocate for optimizing into the unknown, and implement options "in case of". I say we should be thoughtful of not locking users into the defaults we decide to implement. There's a difference between proactively investigating how to enable imaginary use cases, and evaluating the general impact that something we have decided to do will have.

And we should probably do that along the way, as we go. The "maintenance" cost will be ten-fold if we do that as a afterthought and have to track back everything we need to make optional. It's a bit like tests, you've only got once chance to do it at the same time as implementation, when you're fully into the problem at hand.

Also, if someone comes up with an issue asking for an option to unlock a behaviour we do have inadvertently locked, we shouldn't say them to back off "just because", without even considering it. Now, if the thing is gonna increase our maintenance costs inconsiderately or make the tool more complicated for everyone else, that might be valid reasons for declining.

Anyway, we're arguing in a vacuum here... The only concrete question we have here, in this issue, is whether we should have warnings about things we polyfill in jsdom, and my opinion is that it would probably be best to have them (off by default). Because SSR is a sensitive machinery, and some people are bound to run into weird issues. Helping them diagnose the root cause seems sensible to me. It's a good intention for them, and it will garner better feedback for us. Here again, adding the warnings along as we add the polyfills will be cheaper than doing it later.

[Wolfr]

I like the civil, thoughtful discussion, I think we're definitely on the same page. However, I am going to avoid making this issue longer by adding more thoughts :)

[jakobrosenberg]

Sorry for the late post. I might have saved you both some time.

Here are a few clarifications.

1. Routify already scrolls to the top by default
   (Not just the window, but every ancestral element of the replaced component (as we can't make predictions about whether developers locate their main page directly in the window (I often don't)))
2. Routify's scroll already has an opt out
   While the default scroll behavior traverses through each parent and scrolls it to the top, it's easy to optionally break this chain at any point.

<div data-routify="scroll-lock">
<!-- content you don't want scrolled back to the top -->
</div>

3. Routify is already using the element && element.scrollTo check to prevent the scrollTo error
   15c510e

I think we're all on the same page. We want to solve as many challenges as possible while burning as few functionality bridges as possible. For instance, this is why certain issues like service workers have yet to be implemented. It's easy to solve, functionality-wise, but it's hard to solve in a way that takes all the pitfalls into account.

As for warnings or notifications in SSR, we need to inform people in one way or another that SSR could render different from the browser. Take Svelte's store/transition bug. There's a very clear limitation of how you can't use stores and transitions in the same app, but nowhere is that mentioned. Not in the docs, not in the logs, nowhere. Instead you have a broken app and not a single error to go by. In a large app, that's a total nightmare.

Even just a subtle list of noops used could be helpful. Or we could export a log and link to it at the end of the SSR function.

26 pages generated. Read the log

[Wolfr]

I tested it on the latest version of my site and indeed, I did not need the extra code.

Forgive me my ignorance, but what is noops in your last message @jakobrosenberg ?

Your message or part of it seems like it is good for the docs! I will add it to Notion.

Feeel free to close this issue.

[Wolfr]

For docs on scrolling, I rewrote your sentences to not be directed at me but at a general audience: see https://www.notion.so/routifydocs/Scrolling-behavipr-07f7e686e6d54de193e3458908243fa9

[rixo]

noop = no operation

const noop = () => {}

[rjsandman]

Hi, The only reference I have to window is using svelte:window in order to get the scrollY position but I'm getting this error on build. This error doesn't exist in dev mode.

[routify] Exporting to: dist
Error: Not implemented: window.scrollTo

[jakobrosenberg]

@Sandman18 This should be fixed in the latest version of Routify.

If you're using scripts like preview-build or deploy:* make sure to update their dependencies.
roxiness/routify-starter@3743e31