Port config forms JS codebase to TypeScript/spina.

Review Request #13140 — Created July 13, 2023 and submitted

Information

Djblets
release-4.x

Reviewers

This change ports the JavaScript side of the config forms app to use
TypeScript and spina.

For the most part this is a very straightforward port. The only annoying
thing was I discovered that the ListItem implementation allowed
passing in an actions parameter which was treated as an option arg,
but was actually being passed in as part of the attributes (despite not
being an attribute on the model). This was only used inside of our unit
tests (which have themselves been updated to use setActions), but I've
added some compatibility code to emit a deprecation warning just in the
very unlikely case that someone was using this.

Ran js-tests.

Summary ID
Port config forms JS codebase to TypeScript/spina.
This change ports the JavaScript side of the config forms app to use TypeScript and spina. For the most part this is a very straightforward port. The only annoying thing was I discovered that the `ListItem` implementation allowed passing in an `actions` parameter which was treated as an option arg, but was actually being passed in as part of the attributes (despite not being an attribute on the model). This was only used inside of our unit tests (which have themselves been updated to use `setActions`), but I've added some compatibility code to emit a deprecation warning just in the very unlikely case that someone was using this. Testing Done: Ran js-tests.
67d63b3d82d46d0d02a1a17da7d20ed28ba7d5e1
Description From Last Updated

I think this should say "This operates just like a..."?

maubinmaubin

Should say "stores" instead of "stored".

maubinmaubin

4.0? Same below.

chipx86chipx86

suite actually comes from us.

chipx86chipx86

Should use double ticks around "template"

maubinmaubin

I think we need to map this to an explicit type mapping strings -> strings for subclasses to extend, otherwise …

chipx86chipx86

Should document this and any other instance variables.

maubinmaubin

Can we add docs to these? The public ones at least.

chipx86chipx86

It'd be nice to have a type for this that can be imported.

chipx86chipx86

Since this is for returning any arbitrary mapping of data for use during render, I don't think we want unknown. …

chipx86chipx86

This should end with ,, and ) { on the next line.

chipx86chipx86

Missing docs.

chipx86chipx86

Can we add docs for these?

chipx86chipx86

Can we add docs for at least public variables?

chipx86chipx86

Let's follow the pattern in Review Board and have a tests/index.ts in each {models,views}/tests/ directory, and just import that directory. …

chipx86chipx86

Can you decorate Backbone.Collection with :js:class?

chipx86chipx86

I think these have to use 1. and 2. to format correctly in ReStructuredText.

chipx86chipx86

Can you decorate fetch with :js:meth?

chipx86chipx86
maubin
  1. 
      
  2. I think this should say "This operates just like a..."?

  3. Should say "stores" instead of "stored".

  4. Should use double ticks around "template"

  5. Should document this and any other instance variables.

  6. 
      
chipx86
  1. 
      
    1. Just going to remove this for now until I come up with a solution.

  2. I think we need to map this to an explicit type mapping strings -> strings for subclasses to extend, otherwise it'll be limited to these values.

  3. Can we add docs to these? The public ones at least.

  4. Since this is for returning any arbitrary mapping of data for use during render, I don't think we want unknown. I think we want an interface that maps any string key to any value.

  5. This should end with ,, and ) { on the next line.

  6. Can we add docs for these?

  7. Can we add docs for at least public variables?

  8. djblets/static/djblets/js/tests/index.ts (Diff revision 1)
     
     
     
     
     
     

    Let's follow the pattern in Review Board and have a tests/index.ts in each {models,views}/tests/ directory, and just import that directory.

    For example, Review Board's js/tests/index.ts has:

    import '../common/models/tests';
    import '../common/resources/collections/tests';
    import '../common/resources/models/tests';
    import '../common/views/tests';
    import '../reviews/models/tests';
    import '../reviews/views/tests';
    import '../ui/views/tests';
    
  9. 
      
david
chipx86
  1. 
      
  2. It'd be nice to have a type for this that can be imported.

  3. Can you decorate Backbone.Collection with :js:class?

  4. I think these have to use 1. and 2. to format correctly in ReStructuredText.

  5. Can you decorate fetch with :js:meth?

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

Status: Closed (submitted)

Change Summary:

Pushed to release-4.x (938d76c)
Loading...