SSR problem in vue.draggable context

[David-Desmaisons]

Problem:

Hello @owen-m1 , version 1.1.0.0 of sortable.js introduced a call to navigator global property.
This is breaking SSR usage of vue.draggable: building the template on the server-side using node (none browser context).

I tried a fix to require sortable.js only in browser context (SortableJS/Vue.Draggable#724), but it seems

• not to cover all scenarios (see discusrion on Unable to support server-side rendering Vue.Draggable#724)
• to introduce regressions (Non webpack usage Vue.Draggable#731, Integration test using vuedraggable breaks in v2.23.1 Vue.Draggable#730, cannot resolve sortablejs when using rollup Vue.Draggable#728)

I can not think of another fix on vue.draggable side thougth.
Could it be possible to check for navigator early in sortable.js to avoid to raise exception when navigator is not defined?

[transtone]

SortableJS/Vue.Draggable#732

[owen-m1]

I am trying to figure out a solution but it does not make sense to me since the older versions of sortable had the exact same reference to the navigator. The only difference is that in the older versions, Sortable threw an error if the window was not defined, but that is still an error so I don't understand how SSR ever worked with Sortable.

[David-Desmaisons]

The difference should be the code executed just by calling import vs the code executed by the new Sortable.
Vue.draggable is calling the new Sortable only when window is defined.
That solution was working till version 1.9.0 of Sortable.js.
Now the problem is happening during the import.

[owen-m1]

In the previous versions (such as 1.9.0), the navigator code was executed when Sortable was imported. You can see the code here. It is not within the Sortable function but within the UMD function that is called when the script is loaded.

[David-Desmaisons]

That's weird

[David-Desmaisons]

In version 1.9.0, you had this early return:

})(function sortableFactory() {
	"use strict";

	if (typeof window === "undefined" || !window.document) {
		return function sortableError() {
			throw new Error("Sortable.js requires a window with a document");
		};
	}

Could you re-introduce it in version 1.10?

[owen-m1]

@David-Desmaisons I have done this in commit 5fccf71.
Please build Sortable and test it or ask the people who reported this bug to test it, just to make sure it actually fixed the problem.

[princeofnubia]

"TypeError: u is not a constructor"
This is what i am seeing

[David-Desmaisons]

@David-Desmaisons Could it be possible to release a new version with these changes?

[princeofnubia]

I am using it in vue-electron it not coming up

[princeofnubia]

it seems it can only work on browser environment

[princeofnubia]

Error in mounted hook: "TypeError: Sortable is not a constructor"

[owen-m1]

@David-Desmaisons Is it confirmed that it fixes the SSR issue?

[David-Desmaisons]

@owen-m1 I will perform some manual test with the fixed version locally. If everything runs OK, I will let you know.

[Tarrask]

Hi Owen, I've tested the fix, and it doesn't work for me. The problem is that there is quite a few browser specific code executed as soon as someone import/require this library.

I'm using Vue.Draggable that require Sortable. The import is made on serverside and on clientside. But the Sortable constructor is only called in mounted function, which is only executed on the client. I tried to move all the browser specific code in the constructor instead of the root of the module and it works.

I've a commit with the changes Tarrask@8ff64de that works in my case. It's a bit to early to submit a merge request as I don't know yet if it break anything else. For exemple, if you call the constructor more than one time, it will register some listeners twice.

[owen-m1]

@Tarrask Ok, so Sortable never worked for SSR, even before version 1.10.0. That makes more sense.

[Tarrask]

@owen-m1 Exactly, that what I found. But importing it should be harmless.

Actually, a very simple test is with a small js script that just require the library, eg: const Sortable = require('Sortable'); when executing with node, it should not throw anything. It was ok with the 1.9.0 but throw a ReferenceError exception with the 1.10.0. And with the latest it throws the custom exception from windowCheck.js

[owen-m1]

@Tarrask Thank you, I see the issue now. Sortable never supported SSR, but if you imported Sortable without a window object, it would only throw an error when you called the Sortable constructor.

[David-Desmaisons]

@owen-m1 to clarify: sortable by itself did not support SSR but vue.draggable do.

Mainly vue.draggable is avoiding to run any sortable code in a node context.
The problem in version 1.10.0 is that the simple fact of importing Sortable in a node context will throw an exception.

[stuta]

Create file test.js containing:
require('sortablejs');

Run in command line node test.js

[unairubio]

I have traced some refresh issues back to this, what's the ETA of the fix? Thanks!

[owen-m1]

@UnaiRa @stuta Please try building commit 576e335 and tell me if it fixes the problem. I just made it so that Sortable will not throw any errors on import if the window or document is not defined. Obviously it won't work properly unless it is imported in a browser context, but this has always been the behaviour of Sortable.

[Tarrask]

@owen-m1 for me, it works

[unairubio]

@owen-m1 Great, is this making it to a RC anytime soon?

[owen-m1]

@UnaiRa I will release 1.10.1 tonight.

[serdarkok]

Thanks @owen-m1 , look forward to new release.

[owen-m1]

@David-Desmaisons @serdarkok @UnaiRa @Tarrask 1.10.1 is released

[David-Desmaisons]

Thanks!!! I will release a new version of Vue.draggable by Today

[David-Desmaisons]

Version 2.23.2 released! Thanks @owen-m1 and @Tarrask