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 fix base path #7136

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

dragonzhao93
Copy link

💻 变更类型 | Change Type

  • ✨ feat
  • 🐛 fix
  • ♻️ refactor
  • 💄 style
  • 👷 build
  • ⚡️ perf
  • 📝 docs
  • 🔨 chore

🔀 变更说明 | Description of Change

适配NEXT_PUBLIC_BASE_PATH

📝 补充信息 | Additional Information

Copy link

vercel bot commented Mar 25, 2025

Someone is attempting to deploy a commit to the LobeChat Desktop Team on Vercel.

A member of the Team first needs to authorize it.

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Mar 25, 2025
@lobehubbot
Copy link
Member

👍 @dragonzhao93

Thank you for raising your pull request and contributing to our Community
Please make sure you have followed our contributing guidelines. We will review it as soon as possible.
If you encounter any problems, please feel free to connect with us.
非常感谢您提出拉取请求并为我们的社区做出贡献,请确保您已经遵循了我们的贡献指南,我们会尽快审查它。
如果您遇到任何问题,请随时与我们联系。

@dosubot dosubot bot added the 🐛 Bug Something isn't working | 缺陷 label Mar 25, 2025
Copy link
Contributor

gru-agent bot commented Mar 25, 2025

TestGru Assignment

Summary

Link CommitId Status Reason
Detail fa64d43 🚫 Skipped No files need to be tested {"src/app/(backend)/api/auth/[...nextauth]/route.ts":"target file(src/app/(backend)/api/auth/[...nextauth]/route.ts) not in work scope \n include: src/utils,src/server,src/database,src/services,src/store,packages//src/ \n exclude: node_modules,/*.test.ts,/.test.tsx,**/.spec.ts,/*.spec.tsx,/.d.ts,**/.test.js,/*.spec.js,src/database/server","src/app/(backend)/trpc/edge/[trpc]/route.ts":"target file(src/app/(backend)/trpc/edge/[trpc]/route.ts) not in work scope \n include: src/utils,src/server,src/database,src/services,src/store,packages//src/** \n exclude: node_modules,/*.test.ts,/.test.tsx,**/.spec.ts,/*.spec.tsx,/.d.ts,**/.test.js,/*.spec.js,src/database/server","src/app/[variants]/(auth)/next-auth/signin/AuthSignInBox.tsx":"target file(src/app/[variants]/(auth)/next-auth/signin/AuthSignInBox.tsx) not in work scope \n include: src/utils,src/server,src/database,src/services,src/store,packages//src/** \n exclude: node_modules…

Tip

You can @gru-agent and leave your feedback. TestGru will make adjustments based on your input

@arvinxx
Copy link
Contributor

arvinxx commented Mar 25, 2025

@dragonzhao93 不好意思,这个 PR 在我看来算不上轻量化的变更,因为涉及到了所有需要做路径跳转或者请求的 URL ,这意味着未来任何一个新增的特性或者页面都要添加这样的前缀,这会大大增加我们的项目维护成本,这不是我期望看到的。这也是为什么我们 V2 准备完全废弃 basePath 的原因。

我期待的轻量化变更是在类似 Middleware 那样的维度通过一个很轻巧的实现来做到 basePath 的重写,如果是这样的解决方案我会考虑合并。

最后,还是非常感谢你的尝试~ 期待能看到更加理想的解决方案!

@lobehubbot
Copy link
Member

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


@dragonzhao93 Sorry, this PR is not a lightweight change in my opinion, because it involves all URLs that need to be redirected or requested. This means that any new feature or page in the future will have to be added with such a prefix, which will greatly increase the maintenance cost of our project, which is not what I expect to see. This is also why we V2 is preparing to completely abandon basePath.

The lightweight change I'm looking forward to is to achieve basePath rewrite through a very light implementation in a dimension like Middleware. If this is the solution I would consider merging.

In addition, thank you very much for your attempt~ I look forward to a more ideal solution~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Something isn't working | 缺陷 size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants