Convert apiUtils to TypeScript.

Review Request #14157 — Created Sept. 11, 2024 and updated

Information

Review Board
master

Reviewers

This change converts our API methods to TypeScript. Our apiUtils file
has a few methods, primary among them being apiCall. This change adds
typing for everything, and cleans up some old cruft. It does not yet
make any changes to the internals of how API requests work.

In order to facilitate using jasmine for unit tests. these methods are
all exported under a new top-level object called API.

  • Ran js-tests.
  • Smoke tested the UI and verified that things still worked correctly.
Summary ID
Convert apiUtils to TypeScript.
This change converts our API methods to TypeScript. Our apiUtils file has a few methods, primary among them being `apiCall`. This change adds typing for everything, and cleans up some old cruft. It does not yet make any changes to the internals of how API requests work. In order to facilitate using jasmine for unit tests. these methods are all exported under a new top-level object called `API`. Testing Done: - Ran js-tests. - Smoke tested the UI and verified that things still worked correctly.
a657224ac9ce148f81fce048e852172f42ff500f
Description From Last Updated

I don't see where we provide compatibility stubs for the old function names in the RB namespace.

chipx86chipx86

call is a reserved function name. We'll need to call this something else.

chipx86chipx86

I think we want to swap these, given alphabetizing of module names (resourceCollection.ts has it that way).

chipx86chipx86

This can be Record<string, unknown> I think.

chipx86chipx86

While we're changing this, we really should use _.template and gain smart text escaping, since we failed at that originally.

chipx86chipx86

Can you swap these? Alphabetical order.

chipx86chipx86
There are no open issues
chipx86
  1. 
      
  2. Show all issues

    call is a reserved function name. We'll need to call this something else.

    1. Maybe reserved is the wrong word, but it has special meaning for function invocation (functionName.call(this, ...)), and feels really confusing in that way. I don't think it's a great name to use for a function, as API.call then looks like a standard function .call() syntax on API, rather than being our own API call function.

      Maybe we should just call this request or something, if we're modernizing API invocation calls anyway?

  3. 
      
david
chipx86
  1. 
      
  2. Show all issues

    I don't see where we provide compatibility stubs for the old function names in the RB namespace.

    1. That happens in reviewboard/static/rb/js/common/utils/index.ts

  3. Show all issues

    I think we want to swap these, given alphabetizing of module names (resourceCollection.ts has it that way).

  4. Show all issues

    This can be Record<string, unknown> I think.

  5. reviewboard/static/rb/js/common/utils/apiUtils.ts (Diff revision 2)
     
     
     
     
     
     
    Show all issues

    While we're changing this, we really should use _.template and gain smart text escaping, since we failed at that originally.

  6. reviewboard/static/rb/js/common/utils/apiUtils.ts (Diff revision 2)
     
     
     
     
     
     
    Show all issues

    Can you swap these? Alphabetical order.

  7. 
      
david
Review request changed
Commits:
Summary ID
Convert apiUtils to TypeScript.
This change converts our API methods to TypeScript. Our apiUtils file has a few methods, primary among them being `apiCall`. This change adds typing for everything, and cleans up some old cruft. It does not yet make any changes to the internals of how API requests work. In order to facilitate using jasmine for unit tests. these methods are all exported under a new top-level object called `API`. Testing Done: - Ran js-tests. - Smoke tested the UI and verified that things still worked correctly.
201e9d99dcada845bdde4bc279628f00f18b1eb9
Convert apiUtils to TypeScript.
This change converts our API methods to TypeScript. Our apiUtils file has a few methods, primary among them being `apiCall`. This change adds typing for everything, and cleans up some old cruft. It does not yet make any changes to the internals of how API requests work. In order to facilitate using jasmine for unit tests. these methods are all exported under a new top-level object called `API`. Testing Done: - Ran js-tests. - Smoke tested the UI and verified that things still worked correctly.
a657224ac9ce148f81fce048e852172f42ff500f
Diff:

Revision 3 (+872 -452)

Show changes

reviewboard/staticbundles.py
reviewboard/static/rb/js/accountPrefsPage/views/oauthApplicationsView.ts
reviewboard/static/rb/js/accountPrefsPage/views/oauthTokensView.ts
reviewboard/static/rb/js/common/collections/baseCollection.ts
reviewboard/static/rb/js/common/resources/collections/repositoryCommitsCollection.ts
reviewboard/static/rb/js/common/resources/collections/resourceCollection.ts
reviewboard/static/rb/js/common/resources/models/baseResourceModel.ts
reviewboard/static/rb/js/common/resources/models/draftResourceChildModelMixin.ts
24 more

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
maubin
  1. Ship It!
  2. 
      
Loading...