feat(rum-react): use correct path when route is path array

[vigneshshanmugam]

• fix Support path array #702
• Uses the current path from router history if the path is provided as an array. Since the component is re rendered on other path mount, we wont create transaction as its the correct behaviour.

[vigneshshanmugam]

path array is supported from version 5, so updated to latest to test. We dont use any breaking changes so should be good here.

[apmmachine]

💚 Build Succeeded

Expand to view the summary

Build stats

• Build Cause: [Pull request #800 updated]

• Start Time: 2020-06-15T10:15:33.232+0000

• Duration: 70 min 33 sec

Test stats 🧪

Test	Results
Failed	0
Passed	938
Skipped	10
Total	948

Steps errors

Expand to view the steps failures

• Name: Bundlesize

  • Description: #!/bin/bash set -o pipefail npm run bundlesize|tee bundlesize.txt

  • Duration: 1 min 32 sec

  • Start Time: 2020-06-15T10:20:35.752+0000

  • log

[codecov-commenter]

Codecov Report

Merging #800 into master will increase coverage by 0.31%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #800      +/-   ##
==========================================
+ Coverage   92.26%   92.58%   +0.31%     
==========================================
  Files          50       51       +1     
  Lines        2341     2535     +194     
  Branches      488      553      +65     
==========================================
+ Hits         2160     2347     +187     
- Misses        178      185       +7     
  Partials        3        3              

Impacted Files	Coverage Δ
packages/rum-core/src/index.js	100.00% <ø> (ø)
packages/rum-core/src/bootstrap.js	66.66% <100.00%> (+51.28%)	⬆️
packages/rum-core/src/common/polyfills.js	100.00% <100.00%> (+33.33%)	⬆️
packages/rum-core/src/common/url.js	98.61% <100.00%> (ø)
packages/rum-core/src/common/utils.js	94.90% <100.00%> (+1.31%)	⬆️
.../src/performance-monitoring/transaction-service.js	92.09% <100.00%> (-0.29%)	⬇️
...rum-core/src/performance-monitoring/transaction.js	100.00% <100.00%> (ø)
packages/rum/src/apm-base.js	99.05% <100.00%> (ø)
packages/rum/src/index.js	100.00% <100.00%> (ø)
packages/rum-react/src/get-apm-route.js	97.05% <0.00%> (-2.95%)	⬇️
... and 7 more

[hmdhk]

This feature seems very specific to ApmRoute so I think it would be easier if getTransactionName was implement in ApmRoute and send the correct name to withTransaction (would also avoid using getTransactionName)

I also considered if this would be useful for withTransaction alone, but I don't think there's a valid use-case for that. WDYT?

[vigneshshanmugam]

@jahtalab We will not be able to capture the current matched route in the ApmRoute component using any react lifecycle or hook since the path matching algorithm relies inside the rendering of Route component
https://github.com/ReactTraining/react-router/blob/21a62e55c0d6196002bd4ab5b3350514976928cf/packages/react-router/modules/Route.js#L38-L42

So the only way I found was to compute the matched path inside our higher order component ApmComponent which has access to the passed in props to the component being mounted.
https://github.com/ReactTraining/react-router/blob/21a62e55c0d6196002bd4ab5b3350514976928cf/packages/react-router/modules/Route.js#L64

Hope this explains the reason for doing it inside ApmComponent and not inside ApmRoute. Let me know if you need more details or any ideas.

I also considered if this would be useful for withTransaction alone, but I don't think there's a valid use-case for that.

This is not the use case i was trying to address. So we can eliminate this requirement.

[hmdhk]

Thanks for the background @vigneshshanmugam , My suggestion is to extend the functionality of withTransaciton to be able to change the transaction created through that without leaking the logic from ApmRoute into withTransaction. An example would be, if withTransaction accept a third argument as a callback which has the transaction and props, i.e.:

withTransaction('name','type',(transaction, props)=>{transaction.name = ...})(Component)

This has the added advantage that users can add keys from props to transaction labels if needed.

[vigneshshanmugam]

This has the added advantage that users can add keys from props to transaction labels if needed.

Good suggestion. I like the approach, Let me change the PR to account for it.

[hmdhk]

@vigneshshanmugam, I had a quick look, generally looks good, just a few comments!