feat(extendRoutes): allow relative paths in EditableTreeNode

[robertmoura]

BREAKING CHANGE: allow relative path overrides

Closes #341

• Support relative paths for EditableTreeNode
• Require absolute paths at the base of the tree and the root's children

[posva]

Could you add a test? I would like to see the error reproduced

[robertmoura]

I'm not entirely sure how the trees are structured or created. I had to update the code to check that the parent was the root. Can you think of any cases where this would not work?

[posva]

Do you have a bug reproduction? This doesn't seem to be fixing something 😅
Changing paths with non absolute routes is not supported yet #341

[robertmoura]

Quite right! It's actually more of a feature 😊 #341 is exactly the problem I was trying to solve 😊

[robertmoura]

I think I'm seeing the problem now. Here's a snippet out of TreeNode. The path is assumed to be relative (a path segment) or absolute if there is a path override. Interesting behaviour... There has to be a bit more thought put into this. The way I'd expect this to work would mean that the current behaviour would be broken. Just going to close this for now 😊 I might come up with something later. We'll see 😄

  get path() {
    return (
      this.value.overrides.path ??
      (this.parent?.isRoot() ? '/' : '') + this.value.pathSegment
    )
  }

[robertmoura]

If we were to make a slight change to the behaviour of the TreeNode's fullPath getter I think relative paths could be supported. It would be a breaking change though. Can you see any reasons why this would be a bad idea?

I just need to think a little about what this would mean for the route block and definePage.

[posva]

The fullPath shouldn’t change its behavior, or at least it shouldn’t be needed

[robertmoura]

I'll have a look at where it is being used and see if I can remove it 😊

[robertmoura]

Sorry, I was overcomplicating this a lot. I didn't realise that you could have absolute paths as child routes. I just assumed you had to put an extra / in the URL or something. Does what I'm doing now make more sense?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.61%. Comparing base (75160e0) to head (16807d7).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #431      +/-   ##
==========================================
+ Coverage   73.29%   73.61%   +0.31%     
==========================================
  Files          34       34              
  Lines        5767     5775       +8     
  Branches      581      589       +8     
==========================================
+ Hits         4227     4251      +24     
+ Misses       1530     1514      -16     
  Partials       10       10              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[posva]

Looking good. I will probably give this a review and merge next Monday during the stream

[robertmoura]

Closing in favour of #519