[DRAFT] Demo: store the settings in the url hash

[Florent-Bouisset]

Storing the player settings in the url hash let us refresh the demo page while keeping all the settings previously set.
It's more efficient and comfortable when debugging to not have to reset them manually after each page refresh.

To Do:

• Add a switch/checkbox to disable/enable the storing in url OR let the RxPlayer logo be a tag with href with an empty hash allowing to reset.
• Reloading a preset content will display it as a custom content, we can eventually improve that

[peaBerberian]

Add a switch/checkbox to disable/enable the storing in url OR let the RxPlayer logo be a tag with href with an empty hash allowing to reset.

I would advocate for both:

• the switch would stop the URL from updating but keep it where it was. This would be useful when debugging settings you know are problematic, then playing with them, while being able to reset to the problematic scenario by just refreshing the page
• the link would just reset to the default

[peaBerberian]

I didn't look too much into it yet but have you verified that this code is not sensible to race conditions?

For example, if loadContent is called multiple consecutive times, are we sure that the last call will lead to playback in the end?

Will look more into it, but that's a question I was asking myself reading this

[peaBerberian]

(the issue may already have been there before)

[Florent-Bouisset]

I tried to provoke a race condition but didn't succeed in doing so.
I still have added the code to prevent any race condition, let me know what you think of that because I'm not 100% sure that this is the correct way of doing so.

[peaBerberian]

🤷‍♂️

[Florent-Bouisset]

I have made some changes here, JSON.parse() is not safe if the string is not convertible to a JS object. Therefore I wrapped it in a try/catch.
I have added some checks on the shape of the parsed object, there are quite simple as they only check the "typeof", that would be enough. I didn't went as deep as checking if values are respecting the typescript intersection (maybe it's preferable to use a validating library like Yup or Joi if we want to make such checks?)

I have also handled the case where we stringify/Parse "Infinity'

[peaBerberian]

To be honest I still feel a race condition is technically possible here.

Perhaps we should have a mechanism (useRef?) to check that he content is still the last one wanted?

[peaBerberian]

To reproduce a race condition, you would have a first getLoadVideoOptions responding very slowly (e.g. server certificate behind a slow CDN) and try to load another content without this issue.

Here we might play the second content, then play the first

[peaBerberian]

Oh my bad I didn't see isAlreadyLoading is here to fix this. Though I don't think this is the right solution. We want to play the last content, not the initial one.

[peaBerberian]

Why the need to do that? Is it for type casting?

It looks very dependent of what we expose through the demo's options.

[Florent-Bouisset]

If a user reach the demo page with an URL with a pre-establish config:
http://player.com/#config={foo:bar} if at some point we change the demo, and the expected url is now
http://player.com/#config={toto:tata} then we could have errors.

In others words, the URL cannot be trusted and we have to check if the data in it is coherent when parsing it

[peaBerberian]

OK I get it.

But the downside would be that we have to make sure to update that code each time the shape is changed... And I don't think we'll think to do that.

Maybe we could type arg as ILoadVideoConfig or whatever so that TypeScript warn us if we're exploiting inexistant properties? Even with that I don't know...