Add demo add-on for adding a custom exporter #21
Conversation
There was a problem hiding this comment.
Thanks Rumo, this example is a good way of sparking discussion on this. It highlights some of the issues we'll need to think about to come close to the flexibility of Qt add-ons, such as:
- the ability of add-ons to extend the UI
- a lack of static typechecking support - add-on authors currently would not be able to catch breakages if our JS API changes
| @@ -0,0 +1,27 @@ | |||
| function updateExporters(exporters) { | |||
| for (const exporter of exporters) { | |||
| exporter.isDefault = false; | |||
There was a problem hiding this comment.
That feels like something every add-on author will try to do? What happens when multiple add-ons try to become the default?
There was a problem hiding this comment.
If they do it like this the last one will win; if there are multiple defaults the first one due to how findIndex() works.
There was a problem hiding this comment.
I just wonder if it's useful to expose this - presumably all add-ons will want to do it, and then we might as well make it the default, or leave the defaults alone so things are consistent.
There was a problem hiding this comment.
I hadn't assumed users would have multiple add-ons adding custom exporters enabled at once very often.
How about we leave isDefault customisable, but remove it from the example?
That said, it doesn't really matter to me if we expose this, it's just a bit simpler this way.
There was a problem hiding this comment.
Colud we perhaps make it simpler by making it a wantDefault instead? The standard importers could leave it off, and we could then select the first wantDefault, and fall back on the first importer if none have it set.
But that's assuming we want to expose this. One counterpoint for example: the 'special fields' add-on breaks the import of recent .apkg files. Allowing the defaults to be changed could make it harder for the user to realise that problems are caused by an add-on.
There was a problem hiding this comment.
Ok, no problem, I'll remove the default flag on the next occasion.
|
I guess there's also a race condition, at least hypothetically. The event could already have fired when the add-on code runs. One way to solve it would be to additionally set a global variable. The add-on would have to check the variable and only setup the listener if it's not set, yet. Feels hacky ... If you don't know how to make injected code run after Anki's code, either (and I'm getting the feeling it might in fact be impossible due to SvelteKit's event-driven page loading), it seems we need a new Python hook for that. |
Agreed.
That could work. I wonder if we could cut Python out of the equation though? We may eventually get to the point where we want to run JS add-ons on mobile platforms as well, and avoiding a Python intermediary would make that a little easier in the future. We have control over ts/src/app.html, so we can insert inline scripts or non-async script references there, which would be guaranteed to load before any injected JS. We could use that to expose some global promise(s), which injected code could await. It would be nice if we could do it in a way that doesn't require us touching the small synchronous code each time we want to expose a new hook. WDYT? |
|
Sounds good! I will start looking into it next week. |
|
Sorry for the delay here Rumo. While part of me wonders whether there might be a better way than runtime-require to provide typed exports, I don't currently have a better idea, so following the existing pattern of using runtime-require seems like a safe bet unless anyone else like @abdnh @glutanimate and so on wish to chime in. |
|
(are you planning to have requireAsync similarly-typed to require so that if we end up providing a d.ts file to add-on authors in the future, they'll be able to get the correct types from the required namespace?) |
No worries, I hope for the same lenience. 😉
Yes, just pushed it. |
|
Sorry, this fell off my radar. I'm happy to merge it in as-is if you feel it's ready. A quick question first: how do you imagine doExport to be implemented by an add-on author? Presumably they would need to add a new endpoint to mediasrv, and invoke it in doExport()? A minimal example of this might make things easier for people using this demo as a reference. |
|
I was under the impression you wanted to sort out ankitects/anki#3187 first. If you think it makes sense, I will try to add support for bundled Svelte components to ankitects/anki#3145 and this demo.
I'll look into it. |
|
Sorry for the delay; I just replied on ankitects/anki#3187 |
No description provided.