twisty-player tag doesn't play nice with svelte

[anicolao]

Steps to reproduce the issue

Make a svelte component with the example markup:

<twisty-player
  experimental-setup-anchor="end"
  alg="U M' U' R' U' R U M2' U' R' U r"
></twisty-player>

Observed behaviour

Check the console:

proxy.js:15 [HMR][Svelte] Failed to rerender <+page>
logError @ proxy.js:15
(anonymous) @ proxy.js:410
reload @ proxy.js:406
(anonymous) @ hot-api.js:150
(anonymous) @ client.ts:528
(anonymous) @ client.ts:445
(anonymous) @ client.ts:296
queueUpdate @ client.ts:296
await in queueUpdate (async)
(anonymous) @ client.ts:159
handleMessage @ client.ts:157
(anonymous) @ client.ts:91
proxy.js:19 Error: Cannot get `.alg` directly from a `TwistyPlayer`.
    at err (TwistyPlayerSettable.ts:20:10)
    at get alg [as alg] (TwistyPlayerSettable.ts:30:28)
    at set_custom_element_data (index.mjs:477:29)
    at Object.hydrate [as h] (+page.svelte:34:31)
    at Object.create [as c] (+page.svelte:34:31)
    at Object.create [as c] (+page.svelte:30:12)
    at Object.create [as c] (ClassAdder.svelte:27:51)
    at Object.create [as c] (Div.svelte:5:18)
    at create_component (index.mjs:1820:20)
    at Object.create [as c] (ClassAdder.svelte:2:18)

Expected behaviour

Ideally svelte would be able to construct the page.

Environment

MacOS Chrome

🖼 Screenshots

No response

Additional info

No response

[lgarron]

proxy.js:19 Error: Cannot get .alg directly from a TwistyPlayer.

This is presumably because we can't provide synchronous getters for most TwistyPlayer properties. We throw an error if you try to access them:

https://github.com/cubing/cubing.js/blob/4402dfeb1e562a892c701fd117eaa47991fe91a4/src/cubing/twisty/views/TwistyPlayerSettable.ts#L31-L33

Do you know if Svelte has a pattern for async property access?

[anicolao]

I don't know about the internals of Svelte. As I read the specification, there is no such thing as an async Attr, and Attr is required to have a string member value that can be set and get synchronously.

The easiest way for users to work around the problem in Svelte is to build a Svelte component that wraps TwistyPlayer and essentially re-implements twisty-player or a superset or subset of it against the underlying APIs. For my purposes this seemed like a lot of work so I just threw together a very basic wrapper.

<script lang="ts">
	import { Alg } from 'cubing/alg';
	import { TwistyPlayer } from 'cubing/twisty';
	import { onMount } from 'svelte';

	const twistyPlayer: TwistyPlayer = new TwistyPlayer();
	export let controlPanel = 'none';
	export let scramble = '';
	export let solve = '';
	onMount(async () => {
		let contentElem = document.querySelector('#twisty-content');
		if (contentElem) {
			twistyPlayer.background = 'none';
			twistyPlayer.visualization = 'PG3D';
			if (controlPanel === 'none') {
				twistyPlayer.controlPanel = 'none';
			}
			const model = twistyPlayer.experimentalModel;
			twistyPlayer.experimentalSetupAlg = scramble;
			twistyPlayer.alg = solve;
			twistyPlayer.tempoScale = 4;
			twistyPlayer.backView = 'top-right';
			twistyPlayer.hintFacelets = 'none';
			contentElem.appendChild(twistyPlayer);
		}
	});
</script>

<div id="twisty-content"/>

[lgarron]

I don't know about the internals of Svelte. As I read the specification, there is no such thing as an async Attr, and Attr is required to have a string member value that can be set and get synchronously.

Yeah, twisty-player will listen for HTML attributes, but it will never set them — that would unfortunately hurt performance for no benefit for many use cases.

If you're using something that requires reading attributes, your best bet is indeed to make a wrapper.

[lgarron]

Anyhow, here's where the assumption seems to come from:

Specifically, it assumes that if prop in node then node[prop] doesn't throw, which is... not a crazy assumption. But on the other hand, there's no guarantee that an attribute corresponds to a property like that.

We might be able to avoid this by avoiding the relevant getters in TwistyPlayerSettable. We would be doing that already, except it can cause issues with TypeScript use cases and users who would write bugs if we don't throw an error. Maybe there's another workaround.

[lgarron]

We might be able to avoid this by avoiding the relevant getters in TwistyPlayerSettable.

Nope, turns out that's not an option. 😕

[anicolao]

I am fine if you want to close this. The way I'd understand things, twisty-player is a thing that is sort of like an HTML element, but isn't really obeying the rules of an HTML element ... and therefore shouldn't be used in HTML markup. Perhaps my understanding is flawed, but you can't have an attribute on an HTML element that I can't read, and so there's really no reason to think that twisty-player as currently defined would work except by chance.

[lgarron]

The way I'd understand things, twisty-player is a thing that is sort of like an HTML element, but isn't really obeying the rules of an HTML element

I'll have to object to this. 😉 <twisty-player> is very much an HTML element. It follows all "rules", but the issue is with conventions that are not specifically required by the platform.

Perhaps my understanding is flawed, but you can't have an attribute on an HTML element that I can't read, and so there's really no reason to think that twisty-player as currently defined would work except by chance.

It's important to distinguish between attributes and "properties". <twisty-player> listens for attributes, but never modifies them. So document.querySelector("twisty-player").getAttribute("alg") certainly follows the "rules".

Most HTML elements also implement getters and/or setters corresponding to attributes, usually called "properties" ("props" for short). The getter/setter names don't exactly match the attributes, since they "can't" contain dashes.

Right off the bat, it's not safe to assume that an attribute and its corresponding property will have the same value: video.controls is a boolean, while the corresponding attribute value is "" (or any string). It actually looks like the Svelte issue comes from a check that is meant to account specifically for such properties.

---

TwistyPlayer supports setting properties but not getting them, and will probably do so for the time being. To see an example of this issue, consider the timestamp property. Valid values are number and "smart" values: "start " | "end" | "anchor" | "opposite-anchor".

Even ignoring the "smart" values, it is impossible to tell at element instantiation whether a timestamp is valid — that depends on both the alg and the puzzle (for example, (L' R) can be simultaneous moves for 3x3x3 but not pyra). The <video> tag also has similar concerns, and in fact will adjust the currentTime to be in bounds, once it knows the duration of the video. However, it's not clear whether there is an intuitive place to update a synchronous timestamp property for TwistyPlayer: unlike the browser, we don't control how the rendering matches up with JavaScript execution.

Now, consider a more complex but also fundamental example: .alg. We now support cancellation in .experimentalAddMove() based on the puzzle:

import { TwistyPlayer } from "cubing/twisty";

const player = new TwistyPlayer({ alg: "U" });
player.experimentalAddMove("y'", { cancel: true });
(await player.experimentalModel.alg.get()).alg.log();

This logs d. However, the cancellation would be different for another puzzle. But we cannot afford to load information about all puzzles ahead of time — that would download way more code than needed and slow down every player load across all sites. So we need to get that info asynchronously, and therefore alg cannot be updated synchronously without introducing frustrating race conditions. There are some workarounds, such as requiring any heavy code (e.g. puzzle info in this case) to be passed in synchronously when needed. But I spent a few years trying to figure out if we could make that work, and concluded that it was better to avoid synchronous getters for now.

There are also various related issues, such as:

• Making it predictable whether a given change is reflected synchronously or not. (In the snippet above, if we logged the "raw" alg value from the model before the final line, it might very well be the case that the result depends on whether the 3x3x3 code was loaded ahead of time. This is because the same Promise can resolve asynchronously and synchronously depending on where/when it's being awaited.)
• Distinguishing between a "requested" value and the "actual" value. For example, if an alg is invalid for a puzzle then no alg is actually shown. It's also possible to request "visualization": "PG3D" but actually still have a 2D display, such as for clock right now. Sometimes — as with the timestamp — it makes sense for the player to change a value depending on other properties. But other times — as with the alg or visualization — it makes sense to keep around the "requested" value so it can be used when possible.
• Making it easy for someone using cubing.js can get prop values that are consistent with each other, e.g. a timestamp and a "time range" where the former is always within the latter.

Now, in theory we could make some properties synchronous. But then we'd have to figure out:

• Which properties to allow this for.
• How to make it clear & intuitive which getters this works for.
• How to avoid making any decisions that prevent us from implementing other useful changes in the future. Do we want to give up on puzzle-specific simplifications in the snippet above? Can we give up on all such features that may potentially require asynchronous work in the future? This is at odds with important performance goals.

Unfortunately, implementing a performant custom element like ours is hard when there is a lot of "potential" code to load but little "actual" code to load on any given page. Even from talking to experts in the field, it sounds like there is fundamentally no good solution here, so we're doing the best we can. It's always easier to change async code to sync than the other way around, so all the properties are only accessible async (from the model) for now.

This doesn't cause many issues in practice, except it sounds like Svelte is making some assumptions about when a synchronous getter is available. Ideally, we can find a way for Svelte to avoid this assumption and get them to adopt it. Perhaps they might be amenable to wrapping their check in a try-catch.

[anicolao]

Very possibly you are right and I am wrong. The spec I linked to is the spec of what an "Attr" node is in the DOM, and I'm not aware of any DOM node that corresponds to an "HTML Element Property". In my mind, if I write a piece of markup the DOM I expect to encounter is one which contains a node for the tag and nodes for attr1 and attr2 which are by definition read/write attributes. It's very possible and even likely that this interpretation of how my markup should correspond do the DOM is incorrect.

The bottom line is that <twisty-player /> can't be used with Svelte, and presumably will have similar issues with other major frameworks like React and Vue (neither of which I am currently using because I think Svelte's approach is superior for building fast sites). If adoption of <twisty-player /> is a goal, you'll need to find some way to play nice with all these frameworks.

I'd suggest the best way is either tests or example pages written in the popular frameworks so that you can ensure the code is usable as the bare minimum; ideally you'd have some cast of followers writing real example applications others could refer to as exemplars of how to use cubing.js in real-world contexts.

[lgarron]

The bottom line is that <twisty-player /> can't be used with Svelte, and presumably will have similar issues with other major frameworks like React and Vue (neither of which I am currently using because I think Svelte's approach is superior for building fast sites). If adoption of <twisty-player /> is a goal, you'll need to find some way to play nice with all these frameworks.

Indeed, this also makes me sad, particularly since Svelte itself is very much a framework that "plays nice".

I'd suggest the best way is either tests or example pages written in the popular frameworks so that you can ensure the code is usable as the bare minimum;

Yeah, testing this would be great — we already test against multiple bundlers and JS environments in CI, a good next step would be to have framework compatibility tests as well.

I'm going to leave this open until have have something working with Svelte, and definitely want to add tests once we get there.

ideally you'd have some cast of followers writing real example applications others could refer to as exemplars of how to use cubing.js in real-world contexts.

Indeed! But it's also a double-edged sword, because we can't immediately support every use case. I'm confident we'll get there over time, though, and your issues continue to be helpful in identifying real-world situations that we need to plan how to support.