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 URL building for SPA proxying #43407

Merged

Conversation

varsi94
Copy link
Contributor

@varsi94 varsi94 commented Aug 19, 2022

Fix URL building for SPA proxying

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Fix URL building when proxying to a host where the SPA is not on the root URL

Description

If the SPA proxy is not listening on the root url (e.g. http://localhost:3000/spa), then the path segments are ignored when sending the request to the development server. This path segment is prepended to the URL, if there's any.

Fixes #31229

@ghost ghost added area-runtime community-contribution Indicates that the PR has been added by a community member labels Aug 19, 2022
@ghost
Copy link

ghost commented Aug 19, 2022

Thanks for your PR, @varsi94. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@adityamandaleeka adityamandaleeka added feature-spa area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates and removed area-runtime labels Aug 19, 2022
@mkArtakMSFT
Copy link
Member

@MackinnonBuck please talk to Javier about this if you have any questions, and then review this PR. Thanks!

@Tratcher
Copy link
Member

Tratcher commented Sep 7, 2022

Want to fix #43662 while you're at it? It's likely on the same line.

@Tratcher Tratcher self-requested a review September 7, 2022 20:42
@varsi94
Copy link
Contributor Author

varsi94 commented Sep 7, 2022

@Tratcher Fixed #43662 as well.

@Tratcher
Copy link
Member

Tratcher commented Sep 8, 2022

@MackinnonBuck can you do some manual testing for both of these bugs? It doesn't look like there's any automated tests for this.

@dnfadmin
Copy link

dnfadmin commented Sep 8, 2022

CLA assistant check
All CLA requirements met.

@varsi94
Copy link
Contributor Author

varsi94 commented Sep 8, 2022

@MackinnonBuck can you do some manual testing for both of these bugs? It doesn't look like there's any automated tests for this.

I have added some, you can check. @MackinnonBuck @Tratcher

Copy link
Member

@MackinnonBuck MackinnonBuck left a comment

Choose a reason for hiding this comment

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

Just now getting to this - I have one piece of feedback but otherwise it looks great!

Thanks for your contribution @varsi94!

Copy link
Member

@MackinnonBuck MackinnonBuck left a comment

Choose a reason for hiding this comment

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

🎉

@MackinnonBuck MackinnonBuck merged commit 03ddd91 into dotnet:main Sep 16, 2022
@ghost ghost added this to the 8.0-preview1 milestone Sep 16, 2022
@varsi94 varsi94 deleted the bug/31229-fix-spa-proxy-path-building branch September 17, 2022 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates community-contribution Indicates that the PR has been added by a community member feature-spa
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hosting Multiple (React) SPAs in Asp.Net Core for development.
6 participants