Add classes and types for question class and contract

[sr258]

I'm struggling with creating a content types that derives from H5P.Question and implements the question contract. I can't use the H5PContentType class currently provided, as this class derives from H5P.EventDispatcher. It would be great if the collection also included a class that can be used to easily create types using H5P.Question.

[sr258]

Part of the problem is that your H5PContentType (on which I've based a H5PQuestionContentType class) does not seem to inherit from H5P.ContentType. Thus, isRoot and getLibraryFilePath are missing from the prototype. isRoot is used by H5P.Question, so things don't work.

The way H5P.ContentType is made the base class of all content types is extremely strange to me and I don't really understand it: https://github.com/h5p/h5p-php-library/blob/f3579c0d28205bf34490ee151c07d43a2ffc3507/js/h5p.js#L929

It looks like this line has no effect in your H5PContentType class.

[boyum]

Thank you for looking deeper into these types 🤩 I might have to wait until after the holidays before I have the time to look into this, but it'll be in the back of my mind until then.

[sr258]

No need to rush and happy holidays!

I've looked into this a bit further, and I think it has to do with the fact the build with Vite doesn't transpile to ES5 and leaves ES6 classes as they are. It looks like the line I've mentioned above doesn't work with native ES classes, but only with the JavaScript old function class-style/workaround.

[boyum]

It looks like the line I've mentioned above doesn't work with native ES classes, but only with the JavaScript old function class-style/workaround.

This would be true for any content type created as a class, right? Unless the content type's code is ES5-ified?

I have started adding H5P.ContentType to the type library, but I'm afraid the way content types are initialized through newRunnable is not easily representable in TypeScript. I currently see two issues:

1. Content types might be initialized as a regular JS class (with new, by someone not very familiar with H5P perhaps?), instead of with newRunnable, which means isRoot etc won't be added
2. The content type is a class, but is not transpiled to being an ES5 function, which agains leaves the CT without isRoot etc

We could set the methods as optional and document that they might not be set, but I'd like a better solution if there is one. Do you see any other way forward?

[boyum]

Also there's the note in H5P.ContentType's code:

NOTE that this doesn't actually 'extend' the event dispatcher but instead it creates a single instance which all content types shares as their base prototype. (in some cases this may be the root of strange event behavior)

Most content types I've seen in the wild extend H5P.EventDispatcher on their own, so this comment wouldn't make much sense, but this is really difficult to document in my opinion. I lean towards ignoring it, but I might not understand what it entails.

[sr258]

Yes, I believe that ES6 classes that aren't transpiled down to ES5 won't work properly out-of-the-box. The H5P JS Core is really crying for some modernization...

I believe that H5P.ContentType is really a mix-in (or a weird kind of multi-inheritance) that is added to all content objects created with newRunnable. The H5P core does this with a JQuery extend call, which seems to not work for ES6 classes. I'll be trying to find a way to get jQuery extend work with ES6 classes. They have been widely supported for years, and I'd like to use Lit as a frontend library. It relies on native ES6 classes, so I can't transpile them to ES5. The alternative would be to not use the H5P.Question base class (as it uses isRoot) or to add the methods of H5P.ContentType myself.

(The comment in H5P.ContentType's code is incorrect I think. It doesn't look like a singleton.)

[boyum]

I believe that H5P.ContentType is really a mix-in (or a weird kind of multi-inheritance) that is added to all content objects created with newRunnable. The H5P core does this with a JQuery extend call, which seems to not work for ES6 classes. I'll be trying to find a way to get jQuery extend work with ES6 classes. They have been widely supported for years, and I'd like to use Lit as a frontend library. It relies on native ES6 classes, so I can't transpile them to ES5. The alternative would be to not use the H5P.Question base class (as it uses isRoot) or to add the methods of H5P.ContentType myself.

Ohh this seems bad, but you should be able to implement those functions in your class yourself, right? I believe if h5p-utils's H5PContentType class implemented those, they would be overwritten if newRunnable ever changes, so that would be safe. Perhaps that's the way to go forward.

[boyum]

The H5P JS Core is really crying for some modernization...

YES 😭 I'd give away so much time for free if it meant I could refactor and modernise it.

[boyum]

Closing this as I believe we won't come closer to any conclusion here.