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 navigation when base path is set and validate that option's value #1666

Merged
merged 20 commits into from
Jun 11, 2021
Merged

Fix navigation when base path is set and validate that option's value #1666

merged 20 commits into from
Jun 11, 2021

Conversation

ignatiusmb
Copy link
Member

@ignatiusmb ignatiusmb commented Jun 10, 2021

Closes #1359
Fixes #1476
Closes #1608

Updating the docs in this case would be better than manipulating the paths at runtime. Changing the regex or url slicing solves the problem for dev, but still throws when adapters starts building (I tried). Rather than adding checks for all possible uses at runtime or (worse) changing the config passed from the user, it would be clearer for the user to just make sure that the config is correct.

@ignatiusmb ignatiusmb marked this pull request as draft June 10, 2021 15:53
@ignatiusmb ignatiusmb marked this pull request as ready for review June 10, 2021 15:59
@benmccann
Copy link
Member

Have you seen #1608 ? This PR seems to be editing the same line of code. I'm wondering what you think about the difference between the two PRs

@ignatiusmb
Copy link
Member Author

I wasn't aware of that PR, it also seems to solve different, yet similar issues. Checking for a trailing slash would cover this issue and other cases, but I also think it's better to update the docs and mention it in the error message.

@ignatiusmb
Copy link
Member Author

I've merged the other PR here, they should appear as a co-author. Also updated this PR's description.

@benmccann benmccann changed the title add additional check for paths base config validation Fix navigation when base path is set and validate that option's value Jun 11, 2021
@benmccann benmccann merged commit dc56d3c into sveltejs:master Jun 11, 2021
@benmccann
Copy link
Member

updated some of the text slightly and merged. thank you!

@ignatiusmb
Copy link
Member Author

Thank you, Ben! That must've been a chore

@ignatiusmb ignatiusmb deleted the i1359/kit-paths-base branch June 11, 2021 02:47
@benmccann
Copy link
Member

Nope. It was easy enough

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.

navigate always discard base path Unable to navigate to non-root routes by links
3 participants