Upgrade query-string to v8.1.0

[anshgoyalevil]

Which problem is this PR solving?

• This PR upgrades the query-string dep from v6.3.0 to v8.1.0
• part of: Upgrade React version - LFX Mentorship #998

Description of the changes

• In query-string v6.4.0, null type was introduced. sindresorhus/query-string@49d2203#diff-7aa4473ede4abd9ec099e87fec67fd57afafaf39e05d493ab4533acc38547eb8R36
• Further, in later versions, string[] type is changed to (string | null)[]

How was this change tested?

• locally

Checklist

• I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
• I have signed all commits
• I have added unit tests for the new functionality
• I have run lint and test steps successfully
  • for jaeger: make lint test
  • for jaeger-ui: yarn lint and yarn test

[codecov]

Codecov Report

Patch coverage is 100.00% of modified lines.

Files Changed	Coverage
...s/TraceDiff/TraceDiffGraph/traceDiffGraphUtils.tsx	ø
...es/jaeger-ui/src/components/common/UiFindInput.tsx	ø
.../jaeger-ui/src/components/DeepDependencies/url.tsx	100.00%
...s/jaeger-ui/src/components/SearchTracePage/url.tsx	100.00%
...s/jaeger-ui/src/components/TraceDiff/TraceDiff.tsx	100.00%
...onents/TraceDiff/TraceDiffGraph/TraceDiffGraph.tsx	100.00%
...kages/jaeger-ui/src/model/ddg/GraphModel/index.tsx	100.00%
packages/jaeger-ui/src/utils/tracking/ga.tsx	100.00%

📢 Thoughts on this report? Let us know!.

[yurishkuro]

These changes seem problematic to me. Just because some library returns a different result, you changes a lot of signatures in our code, expanding them to more states. Why do those states make sense on all those places? It reduces strong typing.

I would rather add a util function that wraps the library but returns the same [] signature as before. It doesn't make sense se introducing null everywhere.

[anshgoyalevil]

Totally agree. I didn't think of the wrapper function approach at first. Thanks for that. Making the changes.

However, doesn't the current approach seem more readable as the query-string's function signature matches with what we are expecting our function's parameters to be?

[yurishkuro]

there is a principle "be liberal in what you accept and conservative in what you send". Loosening the type of fields within structs breaks that principle because other places in the code that read the struct now must deal with bigger surface.

On the other hand, loosening the type of function parameters seems to align with this principle, but it's not good either, especially in the existing code base. By loosening function signature when it was already tight and the existing code was already confirming to stricter type you are giving permission to that other code to be more sloppy about the typing.

Implementation-wise, I don't have a strong opinion, it could be a helper function, or just x ?? [] when the library is called.

[anshgoyalevil]

there is a principle "be liberal in what you accept and conservative in what you send". Loosening the type of fields within structs breaks that principle because other places in the code that read the struct now must deal with bigger surface.

On the other hand, loosening the type of function parameters seems to align with this principle, but it's not good either, especially in the existing code base. By loosening function signature when it was already tight and the existing code was already confirming to stricter type you are giving permission to that other code to be more sloppy about the typing.

Implementation-wise, I don't have a strong opinion, it could be a helper function, or just x ?? [] when the library is called.

Learning more daily 🚀 Thanks for the useful info.
Have tried this wrapper approach to modify the function's typecasting for the first time.

[anshgoyalevil]

Superseded by #1794