search

jina-ai / dashboard

Interactive UI for analyzing Jina logs, designing Flows and viewing Hub images
https://dashboard.jina.ai
Apache License 2.0
118 stars 60 forks source link

Feat: immerJS for flows-133 #184

Closed BastinJafari closed 3 years ago

github-actions[bot] commented 3 years ago

This PR closes: #133

codecov[bot] commented 3 years ago

Codecov Report

Merging #184 (aafea3d) into master (28ae12c) will decrease coverage by 0.00%. The diff coverage is 53.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #184      +/-   ##
==========================================
- Coverage   33.30%   33.30%   -0.01%     
==========================================
  Files         133      133              
  Lines        2183     2180       -3     
  Branches      369      369              
==========================================
- Hits          727      726       -1     
+ Misses       1452     1450       -2     
  Partials        4        4
Impacted Files Coverage Δ
src/redux/flows/flows.actions.ts 30.76% <0.00%> (+0.91%) :arrow_up:
src/views/FlowView.tsx 0.00% <0.00%> (ø)
src/redux/global/global.actions.ts 86.66% <82.35%> (ø)
src/redux/flows/flows.reducer.ts 93.91% <94.84%> (+0.75%) :arrow_up:
src/redux/flows/flows.constants.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 28ae12c...aafea3d. Read the comment docs.

BastinJafari commented 3 years ago

There are issues because of the fact that two states exist. One in the flow library and one in redux. I would wait for this PR until we have a new flow library

DavidSanwald commented 3 years ago

@BastinJafari Hey just wanted to congratulate you for choosing immerjs. Personally, I can relate why, especially ambitious and bright devs feel drawn to ImmutableJS or alternatives (I like https://github.com/swannodette/mori but I'm a sucker for everything Clojure). I think immer isn't particularly exciting but it might be the most pragmatic and future-proof solution, which is best for the project. I'm happy to see that the dashboard thrives and is moving in a good direction. Congratulation to the whole team.

@BastinJafari I think you're too good of a developer to choose some library because they are intellectually more satisfying. At the same time, I could imagine, that you sometimes crave purer and more radical things, that feel like an actual step forward. So I can really recommend looking into Elm, ClojureScript (especially OM) or ReasonML, just to tickle your fancy (:

DavidSanwald commented 3 years ago

It's a good PR and there was obviously a lot of work, that went into it. So don't let it become stale. Just run: something like that echo fs.inotify.max_user_watches=524288 | sudo tee -a /etc/sysctl.conf && sudo sysctl -p to increase the number of watchers.

BastinJafari commented 3 years ago

Thanks a lot for the feedback David! I Will look into the stuff you recommended, thank you also for that :D

I didn't merge this PR, because I am implementing it into the change of the flow chart library. We will use https://reactflow.dev/