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: generate valid URL route paths for pages on Windows #331

Merged
merged 1 commit into from
Feb 2, 2022

Conversation

petebacondarwin
Copy link
Contributor

@petebacondarwin petebacondarwin commented Jan 28, 2022

Previously route paths were manipulated by file-system path utilities.
On Windows this resulted in URLs that had backslashes, which are invalid for such URLs.

Fixes #51
Closes #235
Closes #330
Closes #327

@changeset-bot
Copy link

changeset-bot bot commented Jan 28, 2022

🦋 Changeset detected

Latest commit: c4762d6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
wrangler Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@petebacondarwin petebacondarwin changed the title WIP fix: generate valid URL route paths for pages on Windows Feb 1, 2022
@petebacondarwin petebacondarwin marked this pull request as ready for review February 1, 2022 18:46
@threepointone
Copy link
Contributor

No changeset? Probably deserves one.

Previously route paths were manipulated by file-system path utilities.
On Windows this resulted in URLs that had backslashes, which are invalid for such URLs.

Fixes cloudflare#51
Closes cloudflare#235
Closes cloudflare#330
Closes cloudflare#327
@petebacondarwin
Copy link
Contributor Author

No changeset? Probably deserves one.

LOL! I had written one, but I didn't include it in the commit.

Fixed.

Copy link
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good to me, stamping.

@petebacondarwin
Copy link
Contributor Author

Thanks @threepointone - I'll let @GregBrimble take a look before landing this.

@petebacondarwin petebacondarwin added this to the Wrangler 2.0 milestone Feb 1, 2022
Copy link
Contributor

@GregBrimble GregBrimble left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks much cleaner, thanks @petebacondarwin !

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.

Bug: generated routes on windows are incorrectly escaped (404)
3 participants