Migrate to typescript and adjust build step / tests

[bitflower]

Summary

(If you have not already please refer to the contributing guideline as described
here)

• Tell us about the problem your pull request is solving.
• Are there any open issues that are related to this? No.
• Is this PR dependent on PRs in other repos? No.

Open points

• Tests
• React Example
• CI/CD workflows
• .gitignore => dist
• Changelog

Other Information

Hi @daffl and @marshallswain,

as discussed this is the Typescript port of feathers-reactive. On my local machine it compiles and tests are running successfully.

What I'm not sure about is the CI/CD pipeline requirements. I think I'd need your help to get this right. Some goes for creating the changelog with npm run changelog. Do I even not to run this locally?

At the moment I'm still consuming the package from my projects from Github which is why the .gitignore file still allows the dist folder. This will be removed in the next step.

Let me know how to continue!

Thanks,
Matthias

Commit history

commit b4c435c
Author: Matthias Max [email protected]
Date: Fri Jun 2 16:10:34 2023 +0200

export Options

commit 394a49e
Author: Matthias Max [email protected]
Date: Fri Jun 2 16:00:30 2023 +0200

.

commit e424b3e
Author: Matthias Max [email protected]
Date: Fri Jun 2 15:58:34 2023 +0200

ci fixes

commit 4980ac7
Author: Matthias Max [email protected]
Date: Fri Jun 2 15:40:54 2023 +0200

final fixes for os tests

commit 1189639
Author: Matthias Max [email protected]
Date: Fri Jun 2 15:38:34 2023 +0200

fix react tests

commit 3fbf3b1
Author: Matthias Max [email protected]
Date: Fri Jun 2 15:12:02 2023 +0200

fix last test

commit 706c3d2
Author: Matthias Max [email protected]
Date: Fri Jun 2 00:02:28 2023 +0200

clean up files

commit c3a0034
Author: Matthias Max [email protected]
Date: Fri Jun 2 00:02:14 2023 +0200

add dist

commit b808bdc
Author: Matthias Max [email protected]
Date: Thu Jun 1 23:59:18 2023 +0200

and more tests

commit e2f0f9f
Author: Matthias Max [email protected]
Date: Thu Jun 1 23:54:50 2023 +0200

more tests

commit 9d1587b
Author: Matthias Max [email protected]
Date: Thu Jun 1 23:43:07 2023 +0200

more tests

commit c6a6970
Author: Matthias Max [email protected]
Date: Thu Jun 1 23:10:16 2023 +0200

first working test

commit 012f7c6
Merge: 9a1163a 92065ff
Author: Matthias Max [email protected]
Date: Thu Jun 1 14:13:51 2023 +0200

Merge branch 'master' of https://github.com/bitflower/feathers-reactive

commit 9a1163a
Author: Matthias Max [email protected]
Date: Thu Jun 1 14:08:15 2023 +0200

.

commit 77bca1d
Author: Matthias Max [email protected]
Date: Thu Jun 1 09:48:20 2023 +0200

typescript migration first step

commit ad8b925
Author: Matthias Max [email protected]
Date: Wed May 31 23:04:32 2023 +0200

fix(bundling): test 1

commit c75f28e
Author: luketych [email protected]
Date: Tue May 16 14:52:27 2023 -0700

Export changes

commit 92065ff
Author: Matthias Max [email protected]
Date: Mon May 15 16:10:16 2023 +0200

fix(): set main and add module in package.json

commit 3cdc993
Author: Matthias Max [email protected]
Date: Mon May 15 16:06:40 2023 +0200

Updating dist

commit 4e7fea2
Author: luketych [email protected]
Date: Sat May 6 11:26:46 2023 -0700

Update list.js

commit 9220280
Author: luketych [email protected]
Date: Sat May 6 11:26:19 2023 -0700

Update strategies.js

commit 8125d16
Author: luketych [email protected]
Date: Sat May 6 11:26:04 2023 -0700

Update resource.js

commit 7193eaf
Author: luketych [email protected]
Date: Sat May 6 11:15:15 2023 -0700

Update package.json

[bitflower]

Hi @daffl,

First of all, I hope reverting my wrong commit on master is fine - I had accidently linked the remote master to my local typescript-migration branch. If we need to revert somehow differently let me know. I'm not a super git pro though...

Let's discuss the required next steps here.

[daffl]

Awesome, thank you! I'll have a closer look next week but if everything passes and once all dependencies are updated we can totally get this out.

[bitflower]

Sure, let me know if it needs anything else!

[Codeonkey]

Will this version of feathers-reactive work in a angular project?

I was trying to use feathers-reactive but version 0.10.0 throws tons of errors preventing the app to start.

When I downgrade the package the 'watch()' function is not available, is this a common error or am I doing something wrong?

[bitflower]

I really think this should be solved @Codeonkey.

You can actually try yourself already by installing directly from the feature branch like:
npm install "https://github.com/feathersjs-ecosystem/feathers-reactive.git#typescript-migration" --save

This version still has the dist folder included that you normally only get from the npm install.

Would love to have your feedback.

[Codeonkey]

@bitflower Thanks for this great work dude!

It works perfectly and made my life SOOOO much simpler to make my app responsive.

Super excited on how simple this library just made my life.

[Codeonkey]

@bitflower
If I have a query with additional checks and I add this watch function, does the event only get emitted to the watch function if all the conditions were met or does it emit even if the conditions is met and the query makes a request even if the payload is not going to change?

So I can answer this myself just by reading through the docs, the matcher param in the watch function allow us to handle when the event should emit or not

.watch({
            matcher: (query) => (post: Post) => {
              return _.eq(query['belongTo'], post['belongTo'].id);
            },
          })

[Codeonkey]

@bitflower After I added the feathers-reactive library, my FeathersResponse type is not working anymore, to what type is the watch() function converting the response too?

Here is a screenshot of the error.

Am I missing an additional step?

When I add a pipe to the options, the type works fine for that payload, but when I add a pipe after the find(), my type is broken.

Here is my code, what is the purpose of the pipe option in the watch function, can you maybe explain a single use case of when it will be used if it does not alter the return type or anything?

Preferably I would like to remove the pipe after the find and let the pipe option return my action, or I should just understand what alterations the watch() function is doing so that I can convert my types.

[bitflower]

@bitflower If I have a query with additional checks and I add this watch function, does the event only get emitted to the watch function if all the conditions were met or does it emit even if the conditions is met and the query makes a request even if the payload is not going to change?

So I can answer this myself just by reading through the docs, the matcher param in the watch function allow us to handle when the event should emit or not

.watch({
            matcher: (query) => (post: Post) => {
              return _.eq(query['belongTo'], post['belongTo'].id);
            },
          })

Hey @Codeonkey, I'm not so much into the original functionality of the lib. I mainly converted it to TS without changing the feature set. I guess you have to find out yourself by debugging. The least I can provide is that I've also had issues with the mode "smart". I sometimes used "always" which kinda defeats the purpose - I know.

[bitflower]

@bitflower After I added the feathers-reactive library, my FeathersResponse type is not working anymore, to what type is the watch() function converting the response too?

Here is a screenshot of the error.

Am I missing an additional step?

When I add a pipe to the options, the type works fine for that payload, but when I add a pipe after the find(), my type is broken.

Here is my code, what is the purpose of the pipe option in the watch function, can you maybe explain a single use case of when it will be used if it does not alter the return type or anything?

Preferably I would like to remove the pipe after the find and let the pipe option return my action, or I should just understand what alterations the watch() function is doing so that I can convert my types.

If you can provide a minimal repro repo I'm happy to check this out! However I donÄt have the time to re-create this myself. Thanks!

[bitflower]

@bitflower After I added the feathers-reactive library, my FeathersResponse type is not working anymore, to what type is the watch() function converting the response too?

Here is a screenshot of the error.

Am I missing an additional step?

When I add a pipe to the options, the type works fine for that payload, but when I add a pipe after the find(), my type is broken.

Here is my code, what is the purpose of the pipe option in the watch function, can you maybe explain a single use case of when it will be used if it does not alter the return type or anything?

Preferably I would like to remove the pipe after the find and let the pipe option return my action, or I should just understand what alterations the watch() function is doing so that I can convert my types.

Which version of rx do you use? This lib now uses 7.5.6.

[bitflower]

Any news @Codeonkey ?

[daffl]

I had a quick look this week. The code looks good, I can totally make another pass to add more of the type information. My questions in general would be if Vite is necessary in this case. It's a good tool for sure but I learned the hard way that any additional dependency will eventually cause issues. My thoughts were:

• Move to Node's built in test runner. This is something I want to do for all projects eventually. One dependency less to worry about (even Mocha updates turned out to be painful over the years).
• Use the TypeScript compiler to build the distributables (maybe as both, CJS and ESM). Now that package managers are so common place I'm not sure how valuable a standalone build is.

I can have a look at this as well unless there are some other reasons for sticking with Vite.

[Codeonkey]

Any news @Codeonkey ?

@bitflower I haven't touched the sideline project again, I will probably be able to in a week or so. I will give an update as soon as I have one. :)

[bitflower]

@daffl Sure, looking back now I just thought it would be easier to setup with unbuild and vitest. No that the main migration is done we can totally refactor to use standards.

Are you talking about this node test runner?

https://nodejs.org/api/test.html

[daffl]

Yep, that was my thought. Now that there is a test runner built in, essentially making the setup as minimalist as possible with just the plain TypeScript build. Then this repo could also be used as a reference for future migrations. I think the test runner should work with TypeScript via node --test --require ts-node/register

[bitflower]

Will have time next week (was in the middle of project handover). FYI!

[bitflower]

Quick confirmation from within the flight cockpit.

1. We want to move from expect to import assert from 'node:assert/strict';

2. I get more typescript warning from the node runner based tests then from the actual compilation step. Can we suppress fro now or what would be your expectation ?
   

Thoughts @daffl ?

[bitflower]

Ok, I have pushed another big chunk of work!

Almost all tests are passing and vitest is completely removed.

@daffl I have just moved with node:assert/strict'.

Pending:
[ ] React part tests
[ ] Fix 2nd test in index.test.ts
[ ] Move to pure typescript build
[ ] Not sure if test coverage is possible with node::test
[ ] Investigate (node:17781) MaxListenersExceededWarning: Possible EventTarget memory leak detected. 11 abort listeners added to [AbortSignal]. Use events.setMaxListeners() to increase limit error

This is what the test results look like:
▶ feathers-reactive
✔ is CommonJS compatible (124.53975ms)
▶ feathers-reactive (125.582917ms)

▶ feathers-reactive integration
✔ works with a client Feathers app initiating service operations (35.592625ms)
✔ works with a client Feathers app listening for service operations (15.003959ms)
✔ works with multiple client Feathers apps (10.22375ms)
▶ feathers-reactive integration (64.448666ms)

▶ reactive lists
▶ strategy.smart
▶ default
✔ .find is promise compatible (2.8695ms)
✔ lazy execution on subscription (2.585667ms)
✔ .find as an observable (0.74ms)
✔ .create and .find (41.44825ms)
✔ .update and .find (22.697542ms)
✔ .patch and .find (22.511458ms)
✔ .remove and .find (21.503167ms)
✔ .find with .create that matches (22.061459ms)
✔ .find with $sort, .create and .patch (43.315708ms)
(node:17781) MaxListenersExceededWarning: Possible EventTarget memory leak detected. 11 abort listeners added to [AbortSignal]. Use events.setMaxListeners() to increase limit
(Use node --trace-warnings ... to show where the warning was created)
✔ removes item after .update/.patch if it does not match (23.06375ms)
✔ adds item back after .update/.patch if it matches again (22.884ms)
▶ default (226.668834ms)

▶ reset
  ✔ subscriber is notified on reset (21.275041ms)
  ✔ after reset, noise data is removed (62.428417ms)
▶ reset (83.862875ms)

▶ custom id
  ✔ .find is promise compatible (0.588958ms)
  ✔ lazy execution on subscription (0.429375ms)
  ✔ .find as an observable (0.393625ms)
  ✔ .create and .find (42.216375ms)
  ✔ .update and .find (22.058292ms)
  ✔ .patch and .find (22.170333ms)
  ✔ .remove and .find (21.9305ms)
  ✔ .find with .create that matches (22.304417ms)
  ✔ .find with $sort, .create and .patch (43.66975ms)

(node:17781) MaxListenersExceededWarning: Possible EventTarget memory leak detected. 11 abort listeners added to [AbortSignal]. Use events.setMaxListeners() to increase limit
✔ removes item after .update/.patch if it does not match (21.173042ms)
✔ adds item back after .update/.patch if it matches again (22.442292ms)
▶ custom id (219.873709ms)

▶ pagination
  ✔ removes items if the data length is past the limit (21.810042ms)
  ✔ .create updates total (22.267125ms)
  ✔ .remove updates total (21.276083ms)
  ✔ update to matching query updates total (3.720333ms)
  ✔ injects options.pipe into observable chain (single operator) (0.589834ms)
  ✔ injects options.pipe into observable chain (array of operators) (0.417959ms)
  ✔ .find uses caching (0.357958ms)
  ✔ clears cache after unsubscription (0.351791ms)
▶ pagination (71.02825ms)

▶ race conditions
  ✔ patch before create event (22.256125ms)
  ✔ update before create event (21.370458ms)
▶ race conditions (43.743459ms)

▶ strategy.smart (645.351542ms)

▶ strategy.always
▶ default
✔ .find is promise compatible (0.31225ms)
✔ lazy execution on subscription (0.442ms)
✔ .find as an observable (0.243833ms)
✔ .create and .find (42.25325ms)
✔ .update and .find (21.287042ms)
✔ .patch and .find (21.898125ms)
✔ .remove and .find (21.881958ms)
✔ .find with .create that matches (22.25175ms)
✔ .find with $sort, .create and .patch (43.719958ms)
(node:17781) MaxListenersExceededWarning: Possible EventTarget memory leak detected. 11 abort listeners added to [AbortSignal]. Use events.setMaxListeners() to increase limit
✔ removes item after .update/.patch if it does not match (22.323875ms)
✔ adds item back after .update/.patch if it matches again (22.395583ms)
▶ default (219.422208ms)

▶ custom id
  ✔ .find is promise compatible (0.41525ms)
  ✔ lazy execution on subscription (0.355833ms)
  ✔ .find as an observable (0.285875ms)
  ✔ .create and .find (41.440666ms)
  ✔ .update and .find (22.368459ms)
  ✔ .patch and .find (20.933667ms)
  ✔ .remove and .find (20.916917ms)
  ✔ .find with .create that matches (21.304792ms)
  ✔ .find with $sort, .create and .patch (43.581208ms)

(node:17781) MaxListenersExceededWarning: Possible EventTarget memory leak detected. 11 abort listeners added to [AbortSignal]. Use events.setMaxListeners() to increase limit
✔ removes item after .update/.patch if it does not match (21.138125ms)
✔ adds item back after .update/.patch if it matches again (22.363792ms)
▶ custom id (215.536375ms)

▶ pagination
  ✔ removes items if the data length is past the limit (22.018125ms)
  ✔ .create updates total (22.19875ms)
  ✔ .remove updates total (21.240667ms)
  ✔ update to matching query updates total (3.246083ms)
  ✔ injects options.pipe into observable chain (single operator) (0.360833ms)
  ✔ injects options.pipe into observable chain (array of operators) (0.286959ms)
  ✔ .find uses caching (0.203542ms)
  ✔ clears cache after unsubscription (0.266625ms)
▶ pagination (70.020916ms)

▶ strategy.always (505.090583ms)

▶ reactive lists (1150.878375ms)

▶ reactive resources
▶ standard id
✔ methods are still Promise compatible (2.551583ms)
✔ .get as an observable (1.946292ms)
✔ lazy execution on subscription (0.870334ms)
✔ .update and .patch update existing stream (1.51875ms)
✔ .remove emits null (3.589292ms)
✔ injects options.pipe into observable chain (single operator) (0.663083ms)
✔ injects options.pipe into observable chain (array of operators) (0.534583ms)
✔ .get uses caching (0.35725ms)
✔ clears cache after unsubscription (0.688791ms)
▶ standard id (13.401333ms)

▶ custom id on service
✔ methods are still Promise compatible (0.519042ms)
✔ .get as an observable (0.485125ms)
✔ lazy execution on subscription (0.685667ms)
✔ .update and .patch update existing stream (0.914208ms)
✔ .remove emits null (1.802916ms)
✔ injects options.pipe into observable chain (single operator) (0.485917ms)
✔ injects options.pipe into observable chain (array of operators) (0.3845ms)
✔ .get uses caching (0.220708ms)
✔ clears cache after unsubscription (0.357458ms)
▶ custom id on service (6.040708ms)

▶ custom id on params
✔ methods are still Promise compatible (0.500958ms)
✔ .get as an observable (0.410125ms)
✔ lazy execution on subscription (0.351333ms)
✔ .update and .patch update existing stream (0.441458ms)
✔ .remove emits null (2.466625ms)
✔ injects options.pipe into observable chain (single operator) (0.444375ms)
✔ injects options.pipe into observable chain (array of operators) (0.464708ms)
✔ .get uses caching (0.269417ms)
✔ clears cache after unsubscription (0.426041ms)
▶ custom id on params (5.925875ms)

▶ reactive resources (25.931ms)

ℹ tests 95
ℹ suites 17
ℹ pass 95
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 2426.983959

[daffl]

This has now been published as v0.11.0. Thank you again for doing this!