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.

Sign up for GitHub

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

Jump to bottom

Update generator.ts to allow %23 (#) in dynamic urls #10965

Merged
merged 11 commits into from
May 16, 2024
Merged

Update generator.ts to allow %23 (#) in dynamic urls #10965

merged 11 commits into from
May 16, 2024

Conversation

Elias-Chairi
Copy link
Contributor

@Elias-Chairi Elias-Chairi commented May 7, 2024 •
edited
Loading

Changes

  • Allows the server to retrive # in dynamic paths (will need to be %23 on client to be recived)
  • I do have a case in my project where this makes sense.

Testing

  • Tested locally and seems to be fine. I dont know why these checks are in place well enough.
  • Didnt see this beeing a problem in my older project where I used react.

Docs

  • Dont think this need any new doc changes
    /cc @withastro/maintainers-docs for feedback!

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented May 7, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: 059b9f6

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

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

@github-actions github-actions bot added pkg: astro Related to the core `astro` package (scope) semver: minor Change triggers a `minor` release labels May 7, 2024
github-actions[bot]
github-actions bot previously requested changes May 7, 2024
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

@Elias-Chairi Elias-Chairi mentioned this pull request May 9, 2024
Closed
1 task
@bluwy bluwy linked an issue May 9, 2024 that may be closed by this pull request
Unable to use %23 (#) in dynamic url #10983
Closed
1 task
@matthewp
Copy link
Contributor

Please provide a test, thank you!

@Elias-Chairi Elias-Chairi requested a review from ematipico May 14, 2024 06:04
@Elias-Chairi Elias-Chairi requested a review from ematipico May 14, 2024 10:01
ematipico
ematipico approved these changes May 14, 2024
View reviewed changes
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Looks good to me. @matthewp ?

@ematipico ematipico merged commit a8f0372 into withastro:main May 16, 2024
13 checks passed
@astrobot-houston astrobot-houston mentioned this pull request May 16, 2024
Merged
@Elias-Chairi Elias-Chairi deleted the patch-1 branch May 16, 2024 10:57
@y-nk
Copy link
Contributor

y-nk commented Jun 27, 2024

@ematipico do you want me to close my #8516 issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ematipico ematipico ematipico approved these changes

@github-actions github-actions[bot] github-actions[bot] left review comments

Assignees
No one assigned
Labels
pkg: astro Related to the core `astro` package (scope) semver: minor Change triggers a `minor` release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unable to use %23 (#) in dynamic url
4 participants
@Elias-Chairi @matthewp @y-nk @ematipico