Pass a function that doesn't return anything to FileUpload component

[rubenvp8510]

Signed-off-by: Ruben Vargas [email protected]

Which problem is this PR solving?

• Fixes Empty screen when click on a trace loaded from JSON #657

Short description of the changes

• In this PR: https://github.com/jaegertracing/jaeger-ui/pull/622/files a Redux middleware was updated to return the value of next(..) in order to preserve the Promises chain, which I think is correct.

• The problem is that Upload.Dragger and our component FileUploader expect a function that returns nothing (...)=>void and the Upload component which is used by Upload.Dragger expect a function that returns nothing or a custom request, not a Promise, returning a Promise breaks the underling Upload component. Upload.Dragger (and our component FileUploader) has type definition for their parameters and Typescript is suppose to do some type checks here , but the search page was not migrated to TS.

Wondering if may be we should do the migration and enforce the types more to prevent future errors like this one. May be in a subsequent PR.

[yurishkuro]

Thanks for the fix!

Wondering if may be we should do the migration and enforce the types more to prevent future errors like this one. May be in a subsequent PR.

Finishing the migration would be great, but difficult to staff. Could there be a unit test to catch this issue?

[yurishkuro]

there is a linter error in CI

[prettier-lint] $ prettier --list-different '{.,scripts}/*.{js,json,md,ts,tsx}' 'packages/*/{src,demo/src}/**/!(layout.worker.bundled|react-vis).{css,js,json,md,ts,tsx}' 'packages/*/*.{css,js,json,md,ts,tsx}'
[eslint       ] $ eslint --cache 'scripts/*.{js,ts,tsx}' 'packages/*/src/**/*.{js,ts,tsx}' 'packages/*/*.{js,ts,tsx}'
[prettier-lint] packages/jaeger-ui/src/components/SearchTracePage/index.js
[prettier-lint] error Command failed with exit code 1.
[prettier-lint] info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
ERROR: "prettier-lint" exited with 1.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
The command "yarn lint" exited with 1.

[codecov]

Codecov Report

Merging #658 into master will increase coverage by 0.05%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #658      +/-   ##
==========================================
+ Coverage   94.15%   94.20%   +0.05%     
==========================================
  Files         227      228       +1     
  Lines        5906     5923      +17     
  Branches     1484     1489       +5     
==========================================
+ Hits         5561     5580      +19     
+ Misses        305      304       -1     
+ Partials       40       39       -1     

Impacted Files	Coverage Δ
.../jaeger-ui/src/components/SearchTracePage/index.js	86.66% <0.00%> (-1.47%)	⬇️
...ackages/jaeger-ui/src/constants/default-config.tsx	100.00% <0.00%> (ø)
...es/jaeger-ui/src/components/DependencyGraph/DAG.js	100.00% <0.00%> (ø)
...src/components/TracePage/TraceGraph/TraceGraph.tsx	95.00% <0.00%> (ø)
...ckages/jaeger-ui/src/utils/version/get-version.tsx	100.00% <0.00%> (ø)
packages/jaeger-ui/src/components/App/TopNav.tsx	92.59% <0.00%> (+0.28%)	⬆️
...s/jaeger-ui/src/components/common/ErrorMessage.tsx	100.00% <0.00%> (+9.37%)	⬆️

---

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 00abad4...86e7b54. Read the comment docs.

[rubenvp8510]

Thanks for the fix!

Wondering if may be we should do the migration and enforce the types more to prevent future errors like this one. May be in a subsequent PR.

Finishing the migration would be great, but difficult to staff. Could there be a unit test to catch this issue?

Agree, it is difficult at this point, I can do a unit test for this case, my concern is if there is any other places where this could be a potential problem. Well, for now I think we are fine with a unit test here.

[yurishkuro]

Well, for now I think we are fine with a unit test here.

yeah, but it didn't catch the issue with return value.