types-mediawiki icon indicating copy to clipboard operation
types-mediawiki copied to clipboard

Improve API parameter types

Open Derugon opened this issue 11 months ago • 3 comments

(note: some changes in this PR massively break existing modules, without providing any viable alternative or migration process, I'm still slowly working on it but for now I do not consider to re-open this proposal, don't hesitate to re-use it or open new ones)

The goal of this PR is to improve parameter auto-completion of API functions. Various type changes are proposed in this PR, and some of these changes are not backwards compatible, as specified below.

Non-backwards-compatible changes:

  • Fix some parameter interfaces sharing the same name (and being merged together),
    • which is very common for format interfaces, since multiple formats are declared in the same PHP class file (for example json, jsonfm and rawfm).
  • Use ApiParams instead of UnknownApiParams on functions doing an actual API query,
    • since parameters need to be correct here.
  • Make some API parameters required, based on API module information,
    • this includes the action property and various properties of action parameter interfaces,
    • token parameters are left optional for now, since I'm not sure whether these should be required or not, for strictness or ease of use.
  • Make some API parameters required with functions that throw an error when these parameters are not specified,
    • note that this is only based on JS code analysis, and does not take into account alternative parameters (like having to specify either title or pageid parameters with action: "protect", so currently omitting both will erroneously typecheck).
  • Narrow action and format parameter type in specific parameter interfaces,
    • so for example, using ApiBlockParams | ApiUnblockParams only allows action: "block" | "unblock".
  • Remove JSON-format specific parameters from the ApiParams interface,
    • the ApiFormatJsonParams interface specify those parameters, so if we want to use ApiMoveParams with JSON-specific parameters, we can use ApiMoveParams & ApiFormatJsonParams.

Backwards-compatible changes:

  • Sort interfaces to not mix up action and format interfaces,
    • so from top to bottom action > format > list/meta/props.
  • Disable tslint rules on specific lines and not for the entire file, so unintended error cas still be detected.
  • Do not auto-complete API params that API functions already specify,
    • for example api.create takes the page title as argument, so there is no need to specify it in additional params,
    • only the obvious ones have been removed, further work could be put into this since a whole lot of API parameters are redundant for some functions.

Note that to simplify the implementation of some of these changes, the api_params type generator has been significantly changed (mainly to separate data extraction functions from code formatting ones). This implementation is partially shared with the one of #40 (to simplify a potential future merge).

Derugon avatar Mar 18 '24 16:03 Derugon

Some methods of mwn (e.g. parseWikitext) provide a default value for action, but ApiParseParams treats action as a required property, which is an incompatible issue.

E.g. https://github.com/AnYiEE/AwesomeGadgets/commit/edc450c0667c625b75fcb30f4cee6e718098bddc#diff-e5e7386f4d0511dd5de28802496acf9bdf305877daae5388f87443c1f8450779R492

AnYiEE avatar Mar 20 '24 02:03 AnYiEE

At the same time, the "variant" property is missing. https://github.com/qiuwenbaike/QiuwenGadgets/commit/6bd8000af45df69df03323da810baf46bd544046#diff-ad3705a25402295b5651e9eb78f318e7c91737361988b391165eeff64948fb32R12

AnYiEE avatar Mar 21 '24 07:03 AnYiEE

Some websites may still be using Flow, but all types related to Flow have been removed in this change (including optional values ​​of the *contentmodel property of some interfaces). Is it necessary to keep them?

The three interfaces ReadingListsApi* extend from ReadingListsApiQueryReadingListsParams -> ApiQueryParams, but their list property type is number, which is incompatible with OneOrMore<string>, causing lint errors.

Some properties should not strictly limit optional values (such as useskin) because some websites do not use certain skins.

AnYiEE avatar Mar 23 '24 12:03 AnYiEE