Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support non absolute paths in EditableTreeNode #341

Open
jods4 opened this issue Mar 13, 2024 · 8 comments · May be fixed by #519
Open

Support non absolute paths in EditableTreeNode #341

jods4 opened this issue Mar 13, 2024 · 8 comments · May be fixed by #519
Labels
👍 contribution welcome others are welcome to implement/fix this ⚡️ enhancement improvement over an existing feature

Comments

@jods4
Copy link

jods4 commented Mar 13, 2024

I'm using extendRoute to normalize our paths (to camelcase, but our files and folders sometimes have different casing).

Setting route.path sometimes incorrectly logs warnings "Only absolute paths are supported.".
Offending code is here: https://github.com/posva/unplugin-vue-router/blob/main/src/core/extendRoutes.ts#L135

The oversight of this warning is that a route can be a nested child route, and path is only the local path, not the full one. In this instance a relative path is ok (common, even).

@posva
Copy link
Owner

posva commented Mar 13, 2024

Can you share a failing test case?

@posva posva added the ♦️ need repro the issue needs a reproduction for further investigation label Mar 13, 2024
@jods4
Copy link
Author

jods4 commented Mar 13, 2024

/Pages
  parent.vue
  /parent
    child.vue

In this situation, the routing table looks like:

[
  { 
    path: "/parent",
    children: [
      { path: "child" } 
    ]
  }
]

Doing a simple route.path = route.path on the child route will report the warning.

@posva
Copy link
Owner

posva commented Mar 14, 2024

The warning is actually correct: currently, only absolute paths are supported. Ideally, setting the path should work with both absolute and non-absolute paths, but that hasn't been implemented yet. Contribution welcome!

@posva posva added ⚡️ enhancement improvement over an existing feature 👍 contribution welcome others are welcome to implement/fix this and removed ♦️ need repro the issue needs a reproduction for further investigation labels Mar 14, 2024
@posva posva changed the title Incorrect warning "Only absolute paths are supported" Support non absolute paths in EditableTreeNode Mar 14, 2024
@jods4
Copy link
Author

jods4 commented Mar 15, 2024

@posva can you explain a little more where the problem lies? I don't get it.
For nested child routes, paths are relative by default and they work.
How is path.route = path.route not idempotent?
Where would something break if I set a nested child path to a relative value, whereas the default relative path actually works?

@posva
Copy link
Owner

posva commented Mar 15, 2024

It’s because in the build-time hooks to change routes you don’t get access to the route records, you get access to an editable tree node that then gets serialized into a route tree. Therefore some operations are different

@jods4
Copy link
Author

jods4 commented Mar 15, 2024

ok.
It's very surprising and unintuitive that what you read (get route() on the record returns the relative path) does not match the semantics of what you write (set route("..") on the record can't be relative?). I think it'd be worthwhile thinking about how this could be improved.

I suppose a work-around for the time being is building and assigning the full absolute path of the child route. With a bit of glue code it can be done (walk up the parents routes and merge them, being careful if they are relative or absolute).

Copy link
Owner

posva commented Mar 15, 2024

Yeah, it’s not intuitive. It will be improved sooner or later

@robertmoura
Copy link

robertmoura commented Jun 26, 2024

Just for context, I believe the issue comes from the differences between the meaning of path in different contexts. Here's an example in the TreeNode class:

https://github.com/posva/unplugin-vue-router/blob/main/src/core/tree.ts

  /**
   * Returns the route path of the node without parent paths. If the path was overridden, it returns the override.
   */
  get path() {
    return (
      this.value.overrides.path ??
      (this.parent?.isRoot() ? '/' : '') + this.value.pathSegment
    )
  }

  /**
   * Returns the route path of the node including parent paths.
   */
  get fullPath() {
    return this.value.overrides.path ?? this.value.path
  }

You can see that the override is actually overriding the whole path not just the pathSegment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👍 contribution welcome others are welcome to implement/fix this ⚡️ enhancement improvement over an existing feature
Projects
Status: 📋 Backlog
3 participants