search

wikimedia-gadgets / types-mediawiki

TypeScript definitions for MediaWiki JS interface
GNU General Public License v3.0
23 stars 8 forks source link

Changing structure #5

Closed cAttte closed 3 years ago

cAttte commented 3 years ago

hey, what do you think about splitting the current typings under /mw/ into two directories: classes and singletons (or namespaces)? i feel like it will get pretty messy once we have everything in place.

another thing, i think it would be better if all classes and independent methods were split; no having Api and ForeignApi under a single Api file, because that's kind of confusing—i get that ForeignApi barely has any content, but it doesn't make a lot of sense to have multiple classes in a single file.

i can make PRs about both of these if you'd like (and i will be opening quite a few more to integrate more classes), but i just want to know your opinion on this!

siddharthvp commented 3 years ago

another thing, i think it would be better if all classes and independent methods were split; no having Api and ForeignApi under a single Api file, because that's kind of confusing—i get that ForeignApi barely has any content, but it doesn't make a lot of sense to have multiple classes in a single file.

Agreed. Go for it.

hey, what do you think about splitting the current typings under /mw/ into two directories: classes and singletons (or namespaces)? i feel like it will get pretty messy once we have everything in place.

I think there are not many classes - Api, Title, Uri, Map. So not sure if that's worth it. But could be done later I guess if things do get messy.

cAttte commented 3 years ago

okay, i'll PR the first one! also, there are actually quite a few more classes (Feedback, ForeignStructuredUpload, Upload, ForeignRest, ForeignUpload, GallerySlideshow, Message, Rest, from the docs). but for some reason, they're only exposed sometimes. for example, mw.GallerySlideshow only exists when there is a slideshow on the page, like here. (so, once we document these we should probably add warnings in their descriptions). but the directories are not that big of a deal anyways.

cAttte commented 3 years ago

wait, another problem! there are a namespace and class that are case-insensitively equal: notification and Notification. the class isn't typed yet,† ...and practically it could never be because we can't have those two files—unless we put both in a single file (and you know how i feel about that!), or we split classes and namespaces :).† what do you think?

* the class is private, so it's not exposed as mw.Notification; currently, the return types of mw.notify() and mw.notification.notify() are object literals with the class's methods, which is kind of weird. it would make sense for it not to be available, but we should just make it with a private constructor instead, which also has the advantage of being able to use it as a type (and, the disadvantage of not being able to use it at runtime (e.g. x instanceof mw.Notification) even though it is declared, which would be confusing).

† i realized that currently, methods are stored as separate files too. so, we would also split these into a /methods/ directory then, unless we put them in the index declaration (which would be consistent with how its properties are stored), but that would also make the main file quite lengthy, especially after the PR i'm opening later that will add stricter typings to hook().

sorry for the wall of text!

siddharthvp commented 3 years ago

I wouldn't recommend having separate files for private classes, or for notification and Notification as IMO that can become confusing. Since the latter is a private class, we could declare it as an interface - this way it would usable as a type but not in runtime.