Port UserSession to spina.

Review Request #13005 — Created May 3, 2023 and submitted

Information

Review Board
release-6.x

Reviewers

This change ports over the UserSession and associated helpers to spina
and TypeScript.

  • Ran js-tests.
  • Tested review request starring, muting, and archiving, and group
    starring.
Summary ID
Port UserSession to spina.
This change ports over the UserSession and associated helpers to spina and TypeScript. Testing Done: - Ran js-tests. - Tested review request starring, muting, and archiving, and group starring.
875b385029d310e99e16f818074155a90806870f
Description From Last Updated

Oh, I thought we were keeping all these for another version? Same below. For obj and others, can we type …

chipx86chipx86

Can you document these?

chipx86chipx86

When possible, we should use unknown, which is intended to replace any and has some level of type checks. No …

chipx86chipx86

This applies to all tests, current and future. I think we want to use relative paths in tests, in order …

chipx86chipx86
chipx86
  1. 
      
  2. reviewboard/static/rb/js/common/models/userSession.ts (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
    Show all issues

    Oh, I thought we were keeping all these for another version? Same below.

    For obj and others, can we type this to something? Whether a model or an interface or whatever.

    1. I decided to keep it for BaseResource and friends, but these methods are almost certainly only used by us, so keeping compatibility here didn't seem to buy us much.

  3. Show all issues

    Can you document these?

  4. Show all issues

    When possible, we should use unknown, which is intended to replace any and has some level of type checks. No idea if that's an option here.

  5. Show all issues

    This applies to all tests, current and future.

    I think we want to use relative paths in tests, in order to avoid these turning into RB.UserSession and such. Direct access means fewer things we have to export into RB (we can export it in the imported module, but don't have to put it in RB just to test it).

    It also allows us to bundle in tested modules straight into the tests bundle and eventually avoid having to import all bundles into the test runner.

    1. It turns out that for UserSession this doesn't actually work. If some code is getting UserSession from the common bundle, and some is getting it from the test bundle, they'll end up with different instances. I'll go through and use local imports where possible but we probably can't do it wholesale for all tests until a much larger fraction of our code is updated to use imports.

    2. Ah I see. Makes sense. Just reading this now after having made similar comments on other changes. Do with that what you will.

  6. 
      
david
chipx86
  1. Ship It!
  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-6.x (1f555ae)
Loading...