Feature helpers ts #1

Fajfa · 2020-02-04T12:00:15Z

Files to be converted to Typescript

Shared
System
Messaging
Compose

darh

This needs some love.
Plus -- please spend some time studying existing code/functions in this repo (like casters, guards... ) and how they are used.

src/corredor/helpers/shared.ts

darh

Good first step, but let's improve it as much as we can.

Please take a look at tsdoc and how can existing (jsdoc) comments be changed

src/corredor/helpers/shared.ts

src/corredor/helpers/system.ts

src/corredor/helpers/messaging.ts

darh · 2020-02-06T17:08:47Z

src/corredor/helpers/system.ts


 /**
 * SystemHelper provides layer over System API and utilities that simplify automation script writing
 */
 export class System {
-  constructor (ctx = {}) {
+  private SystemAPI: SystemAPI;


This MUST be set, not optional.

(same in other helpers)

Should only the API be set or all the ctx params ?

inspect the code and propose/decide :)

The API and namespace params are required. Everything else is optional.

Scratch that, namespace is optional too.

src/corredor/helpers/system.ts

src/corredor/helpers/messaging.ts

darh · 2020-02-06T17:15:30Z

src/corredor/helpers/messaging.ts

+  private $channel?: Channel;
+  private $message?: Message;
+
+  constructor (ctx: Context) {


You could replace Context with Partial<Messaging>
https://www.typescriptlang.org/docs/handbook/utility-types.html#partialt

I've tried to do this, but i can't get it working. Looked at other code using Partial and tried it that way. No luck.

What did you try and what does not work?

Partial sets all the props to optional, so that isn't too good since we want the API to be set.

Fajfa · 2020-02-10T12:34:11Z

Theres still an error in messaging.ts helper on line 85. Casting to new Message() doesn't yet work, because the type isnt done yet. The line is commented for now.

darh · 2020-02-11T06:35:27Z

src/corredor/helpers/messaging.ts

+}
+
+interface ChannelResponse {
+  [_: string]: unknown;


Is this [string]:unknown really needed?

channel response (as well as any other list responses) are always composed of filter+set.
No other arbitrary props.

(applies to other cases here too)

the [_:string]: unknown in all the responses is there so i dont have to convert from API response to unknown to Channel Response(res as unknown as ChannelResponse). I can instead just do:
res as ChannelResponse

src/corredor/helpers/messaging.ts

src/corredor/helpers/shared.ts

src/corredor/helpers/compose.ts

darh · 2020-02-11T07:39:50Z

src/corredor/helpers/messaging.ts

+  private $channel?: Channel;
+  private $message?: Message;
+
+  constructor (ctx: Context) {


What did you try and what does not work?

darh

almost there.

Fajfa requested a review from darh February 4, 2020 12:00

darh suggested changes Feb 4, 2020

View reviewed changes

src/corredor/helpers/shared.ts Outdated Show resolved Hide resolved

src/corredor/helpers/shared.ts Outdated Show resolved Hide resolved

src/corredor/helpers/shared.ts Outdated Show resolved Hide resolved

darh self-assigned this Feb 6, 2020

Fajfa force-pushed the feature-helpers-ts branch from 7fb622e to 3f35f20 Compare February 6, 2020 15:10

darh suggested changes Feb 6, 2020

View reviewed changes

Fajfa force-pushed the feature-helpers-ts branch from 3f35f20 to a9bd7c4 Compare February 9, 2020 23:39

Fajfa changed the title ~~WIP: Feature helpers ts~~ Feature helpers ts Feb 10, 2020

Fajfa force-pushed the feature-helpers-ts branch from 434dc30 to 1306840 Compare February 10, 2020 12:32

Fajfa force-pushed the feature-helpers-ts branch from 1306840 to c893893 Compare February 10, 2020 12:54

darh reviewed Feb 11, 2020

View reviewed changes

darh suggested changes Feb 11, 2020

View reviewed changes

Fajfa force-pushed the feature-helpers-ts branch from c893893 to 4ef8ed8 Compare February 11, 2020 12:21

Fajfa and others added 14 commits February 11, 2020 15:28

Convert shared.js helper to Typescript

d2ea394

Convert system.js helper to Typescript

94d221b

Convert messaging.js helper to Typescript

c9c555f

Convert compose.js helper to Typescript

9adf967

Polish helpers

2c03aa3

Removed types in @param (tsdocs)

a8dbd1d

Remove eslint-ignore, cleanup @return tsdoc, rename helper classes

3c7b198

Add ts-sinon testing lib

522c599

Remove eslint-disable and fix all issues

cf66f8b

Change helper props to readonly

3a0f91d

User/Role list fn() should not throw error on 0 results

1eec3e3

User/Role list filter is optional arg

32e2086

Fix arg type for addUserToRole

8ca2593

Add system helper tests

8d55f7f

darh force-pushed the feature-helpers-ts branch from 4ef8ed8 to 8d55f7f Compare February 11, 2020 18:15

darh added 2 commits February 11, 2020 20:43

Improve ID handling

a4c3f33

Add tests for shared helper functions

bd83b1f

darh added 4 commits February 11, 2020 20:49

Fix namespace ctor args

955f4ba

Minor cosmetic changes on system helper tests

1c21a53

Compose helper tests migrated

4d53e29

Add (mostly pending) tests for messaging helper

3a9c5c6

darh merged commit 3a9c5c6 into develop Feb 12, 2020

darh deleted the feature-helpers-ts branch February 12, 2020 06:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature helpers ts #1

Feature helpers ts #1

Fajfa commented Feb 4, 2020 •

edited

Loading

darh left a comment

darh left a comment

darh Feb 6, 2020

darh Feb 6, 2020

Fajfa Feb 10, 2020

darh Feb 10, 2020

Fajfa Feb 10, 2020

Fajfa Feb 10, 2020

darh Feb 6, 2020

Fajfa Feb 10, 2020

darh Feb 11, 2020

Fajfa Feb 11, 2020

Fajfa commented Feb 10, 2020 •

edited

Loading

darh Feb 11, 2020

Fajfa Feb 11, 2020 •

edited

Loading

darh Feb 11, 2020

darh left a comment

Feature helpers ts #1

Feature helpers ts #1

Conversation

Fajfa commented Feb 4, 2020 • edited Loading

darh left a comment

Choose a reason for hiding this comment

darh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fajfa commented Feb 10, 2020 • edited Loading

Choose a reason for hiding this comment

Fajfa Feb 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

darh left a comment

Choose a reason for hiding this comment

Fajfa commented Feb 4, 2020 •

edited

Loading

Fajfa commented Feb 10, 2020 •

edited

Loading

Fajfa Feb 11, 2020 •

edited

Loading