[DeAngular][visualization][vislib] remove angular from vislib

[ananzh]

Description

• remove angular import and $scope from Binder class since there is
  no usage in the code

Issue resolved

#2137

Check List

• New functionality includes testing.
  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
    • yarn test:ftr
• New functionality has been documented.
• Commits are signed per the DCO using --signoff

[ashwin-pc]

Can you give more context on why we are doing this change? You have removed the constructor from the Binder class. How did you decide that it wasn't necessary.

[codecov-commenter]

Codecov Report

Merging #2138 (e5060c9) into main (4dd7f14) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2138   +/-   ##
=======================================
  Coverage   67.23%   67.23%           
=======================================
  Files        3100     3100           
  Lines       59566    59564    -2     
  Branches     9063     9062    -1     
=======================================
  Hits        40047    40047           
+ Misses      17332    17331    -1     
+ Partials     2187     2186    -1     

Impacted Files	Coverage Δ
...lugins/vis_type_vislib/public/vislib/lib/binder.ts	58.82% <ø> (+6.19%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[ananzh]

Can you give more context on why we are doing this change? You have removed the constructor from the Binder class. How did you decide that it wasn't necessary.

Hi @ashwin-pc I put the details in the issue (#2137) . Should point it here. Do you think this is good enough?

[ashwin-pc]

Yeah i saw that. My only concern is if $scope is passed in as a side effect. I'm not too familiar with angular and how the data binding $scope is passed into the component. And what this code appears to do is destroy the bind when the scope is destroyed. Have you verified that in both these instances that we never end up with $scope in the constructor?

[ananzh]

#2137

@ashwin-pc For the two uses, I don't see any of them passing the @scope as the parameter to the constructor. There is no side effect or other places to call @scope.

@scope is a middle layer between view (html page) and controller ( to control behavior on the view, like state in react).
destroy is to remove the current scope (and all of its children) from the parent scope. and there is nothing to destroy here since all the scopes are cleaned.

I think it is safe to remove this part.

[kavilla]

#2137

@ashwin-pc For the two uses, I don't see any of them passing the @scope as the parameter to the constructor. There is no side effect or other places to call @scope.

@scope is a middle layer between view (html page) and controller ( to control behavior on the view, like state in react). destroy is to remove the current scope (and all of its children) from the parent scope. and there is nothing to destroy here since all the scopes are cleaned.

I think it is safe to remove this part.

Makes sense to me, probably before merging we can update the pull request (via github) to include these details.

[ashwin-pc]

Yeah sorry for getting back to this so late, thanks for the explanation @ananzh. That makes sense 😄