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

Feature/basename #4459

Closed
wants to merge 12 commits into from
Closed

Feature/basename #4459

wants to merge 12 commits into from

Conversation

yracnet
Copy link

@yracnet yracnet commented Oct 31, 2022

I have implemented the feature described on the #2891.

This include the serverBasename attribute on remix.config.js for declare a prefix to URL

serverBasename default value is ""
fix deploy on path server (basename)
fix the navegation in client side or server side components

@changeset-bot
Copy link

changeset-bot bot commented Oct 31, 2022

⚠️ No Changeset found

Latest commit: 28fd0a1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Oct 31, 2022

Hi @yracnet,

Welcome, and thank you for contributing to Remix!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at hello@remix.run.

Thanks!

- The Remix team

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Oct 31, 2022

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

Copy link
Collaborator

@machour machour left a comment

Choose a reason for hiding this comment

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

Hi @yracnet and thank you for taking the time to tackle this 🙌🏼
Some unwanted commits made it to your PR, could you fix that?

@machour
Copy link
Collaborator

machour commented Oct 31, 2022

Refs #2996

@gerbyzation
Copy link

This would be a great feature to have in remix. If I can help with making this happen let me know!

@yracnet
Copy link
Author

yracnet commented Dec 16, 2022

Thanks, but I dont know what is the next step.

@gerbyzation
Copy link

@yracnet As mentioned in this comment I think they are waiting for some unrelated changes that have snuck in to be removed: #4459 (review) For example decisions/0006-linear-workflow.md is added as part of this PR but doesn't seem related to the scope?

@hemangsk
Copy link

@yracnet , First of all great job, this is such a useful feature.

Do I have your permission to fork this branch and do the required changes (removing unrelated commits) and make the PR merge ready? We really need this feature and incase you're busy, I'll be more than willing to get things moving here 🙏

@yracnet
Copy link
Author

yracnet commented Jan 21, 2023

Hi @hemangsk thanks, I have merged my branch with the las change from dev, I run the test. I will wait to approval my changes. thanks

@gerbyzation
Copy link

@yracnet thanks for updating your branch! Unfortunately there are still unrelated changes in the diff. As @hemangsk and I have offered we're happy to port your work to a clean branch to get this shipped if you don't mind

@yracnet
Copy link
Author

yracnet commented Jan 23, 2023

Thanks, I will send a new pr with the clean branch from dev.

@yracnet
Copy link
Author

yracnet commented Jan 24, 2023

clean branch to #5236

@yracnet yracnet closed this Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants