Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve API parameter types #41

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Derugon
Copy link
Collaborator

@Derugon Derugon commented Mar 18, 2024

(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).

Adrien LESÉNÉCHAL added 4 commits March 18, 2024 15:38
to split data extraction from TS code formatting, and allow further improvements
- make some API parameters required, based on module information,
- infer `action` and `format` parameter value depending on the params interface used
- disable `tslint` rules in specific lines and not globally, so unintended error cas still be detected
- add missing `fm` formats
- fix some interfaces sharing the same name
- sort interface to not mix up "action" and "format" interfaces
- use `ApiParams` instead of `UnknownApiParams` on functions doing an actual API query, since parameters need to be correct here,
- do not auto-complete API params that API functions already specify
- force some API params to be specified, when the functions throw an error if missing
- remove JSON-format specific params, use `ApiFormatJsonParams` instead
@Derugon Derugon changed the title Improve API params types Improve API parameter types Mar 18, 2024
AnYiEE added a commit to AnYiEE/types-mediawiki-renovate that referenced this pull request Mar 20, 2024
feat: add more MediaWiki modules check other authors/commits at wikimedia-gadgets#41
@AnYiEE
Copy link
Contributor

AnYiEE commented Mar 20, 2024

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. AnYiEE/AwesomeGadgets@edc450c#diff-e5e7386f4d0511dd5de28802496acf9bdf305877daae5388f87443c1f8450779R492

@Derugon Derugon marked this pull request as draft March 20, 2024 10:56
@AnYiEE
Copy link
Contributor

AnYiEE commented Mar 21, 2024

At the same time, the "variant" property is missing. qiuwenbaike/QiuwenGadgets@6bd8000#diff-ad3705a25402295b5651e9eb78f318e7c91737361988b391165eeff64948fb32R12

Rewrite API queries to recursively retrieve all submodules, not only main modules and action=query submodules

Not that a submodule hierarchy is built, so each submodule remembers where it comes from. This is not used for now, but will be used to fix some perfix & interface name issues in another commit.
@AnYiEE
Copy link
Contributor

AnYiEE commented Mar 23, 2024

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.

- make interface order deterministic, so alternatives are sorted alphabetically and submodules are next to their root module
- fix action/format modules not being stored correctly in cache
AnYiEE added a commit to AnYiEE/types-mediawiki-renovate that referenced this pull request Mar 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants