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

fix: remove trailing slash to allow deparam with urls that end with a slash #259

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

m-thompson-code
Copy link

Description

I'm working with a team to perform static site generation that works with can-route, and currently, can-route's canRoute_deparam method doesn't handle urls that end with a trailing slash (/) properly. This has become problematic when trying to use express to act as a local static server during development.

This PR updates canRoute_deparam to removing trailing slash (/) from url when trying to deparam for path variables.

Recreate Steps

import route from "can-route"
import "can-stache-route-helpers"

route.urlData = new RoutePushstate()

route.register("{page}", { page: "home" })
route.register("tasks/{taskId}", { page: "tasks" })
route.register("progressive-loading/{loadId}", { page: "progressive-loading" })

route.start()

This allows for urls like: "progressive-loading/moo" which deparams to:

{
  "page": "progressive-loading",
  "loadId": "moo"
}

When the url is: "progressive-loading/moo/" (has trailing slash /), deparams to empty object:

{}

When removing the trailing slash (/) then attempting to deparam works as expected.

Reason to support urls with trailing slash (/)

When serving static pages using express, following their basic static server example looks something like this:

const express = require("express")
const app = express()

app.use(express.static('static_files'))// return all static files from "static_files"
app.use("*", express.static('static_files/404'))// if there's no match, return 404 page

app.listen(8080, function () {
  console.log("Example app listening on port 8080")
})

"/static_files/":

/static_files/index.html
/static_files/tasks/index.html
/static_files/progressive-loading/root/index.html
/static_files/progressive-loading/moo/index.html
/static_files/progressive-loading/cow/index.html
/static_files/404/index.html

@m-thompson-code
Copy link
Author

A workaround to this is to provide doubles for each register:

route.register("{page}", { page: "home" })
route.register("{page}/", { page: "home" })
route.register("tasks/{taskId}", { page: "tasks" })
route.register("tasks/{taskId}/", { page: "tasks" })
route.register("progressive-loading/{loadId}", { page: "progressive-loading" })
route.register("progressive-loading/{loadId}/", { page: "progressive-loading" })

thanks @bmomberger-bitovi for your suggestion

@m-thompson-code
Copy link
Author

Something that is missing from this PR is the ability to generate urls given a trailing /:

for example { page: 'first', action: 'second'} should be able to generate a url "/first/second/" instead of "/first/second"

To avoid breaking changes, this PR should instead add an option to switch into defaulting into this behavior

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant