feat: supports publicPath: auto #1717
Conversation
xusd320
requested review from
sorrycc and
stormslowly
and removed request for
sorrycc
|
Warning Rate limit exceeded@xusd320 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (7)
Walkthrough该拉取请求对 Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
WalkthroughThis pull request introduces support for the Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
xusd320
force-pushed
the
feat/publicPath-auto
branch
from
5064361 to
7ec1f59
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
docs/config.zh-CN.md (1)
631-632: 修改无序列表符号以符合 Markdown 风格指南根据项目的 Markdown 风格指南，无序列表应使用
-而非*。请将第 631 和 632 行的*替换为-。建议应用以下修改：
630 publicPath 配置。注意：有两个特殊值 -631 * `"runtime"`，这意味着它将切换到运行时模式并使用运行时的 `window.publicPath` 作为 publicPath; -632 * `"auto"`，类似 webpack 的 `publicPath: "auto"`。 +631 - `"runtime"`，这意味着它将切换到运行时模式并使用运行时的 `window.publicPath` 作为 publicPath; +632 - `"auto"`，类似 webpack 的 `publicPath: "auto"`。🧰 Tools
🪛 Markdownlint (0.35.0)
631-631: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
632-632: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
docs/config.md (3)
632-633: 需要修正语法错误原文中 "There is two special values" 存在语法错误，应该修改为 "There are two special values"。
-publicPath configuration. Note: There is two special values +publicPath configuration. Note: There are two special values🧰 Tools
🪛 LanguageTool
[grammar] ~632-~632: Did you mean “There are two special values”?
Context: ..."/"publicPath configuration. Note: There is two special values -"runtime", which means that it wil...(THERE_S_MANY)
634-635: 建议改进 runtime 模式的文档说明当前对
runtime模式的说明过于简单。建议补充以下内容：
- 何时使用此模式
- 如何在运行时正确设置
window.publicPath- 提供完整的使用示例
-which means that it will switch to runtime mode and use the runtime `window.publicPath` as publicPath. +which enables runtime configuration of publicPath. When this value is set, Mako will use `window.publicPath` as the base URL for loading assets. This is useful when the base URL needs to be determined at runtime, for example, in micro-frontend applications. + +Example: +```js +// Set the runtime publicPath before loading any assets +window.publicPath = 'https://cdn.example.com/assets/'; +```🧰 Tools
🪛 LanguageTool
[uncategorized] ~634-~634: Loose punctuation mark.
Context: ...ere is two special values -"runtime", which means that it will switch to runt...(UNLIKELY_OPENING_PUNCTUATION)
636-636: 完善 auto 模式的文档说明当前对
auto模式的说明不够详细。建议参考 webpack 文档，补充具体的行为说明。-- `"auto"`, which is just like `publicPath: "auto"` in webpack +- `"auto"`, which automatically determines the publicPath from which the script or module is loaded. This works similar to webpack's `publicPath: "auto"`. For example, if your entry script is loaded from `https://example.com/scripts/main.js`, the publicPath will automatically be set to `https://example.com/scripts/`. + + This is particularly useful when: + - The base URL is not known at compile time + - The assets are hosted on a different domain + - The application is deployed to multiple environments with different base URLs🧰 Tools
🪛 LanguageTool
[uncategorized] ~636-~636: Loose punctuation mark.
Context: ...w.publicPathas publicPath. -"auto", which is just likepublicPath: "auto"`...(UNLIKELY_OPENING_PUNCTUATION)
🛑 Comments failed to post (3)
crates/mako/src/plugins/runtime.rs (2)
40-42:
⚠️ Potential issue修正 Web Worker 环境下的
scriptUrl获取方式在 Web Worker 环境中，
window对象不可用，应该使用self来获取location属性。请将window.location替换为self.location。建议应用以下修改：
- scriptUrl = window.location + ""; + scriptUrl = self.location + "";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (!self.document && self.importScripts) { scriptUrl = self.location + ""; }
44-52:
⚠️ Potential issue修复条件语句的缩进和逻辑错误
在第 44 行至 52 行，条件语句的缩进不正确，可能导致逻辑错误。建议添加花括号并调整缩进，以确保代码逻辑的正确性和可读性。
建议应用以下修改：
if (document.currentScript && document.currentScript.tagName.toUpperCase() === 'SCRIPT') { scriptUrl = document.currentScript.src; - if (!scriptUrl) { - var scripts = document.getElementsByTagName("script"); - if(scripts.length) { - var i = scripts.length - 1; - while (i > -1 && (!scriptUrl || !/^http(s?):/.test(scriptUrl))) scriptUrl = scripts[i--].src; - } - } + } + if (!scriptUrl) { + var scripts = document.getElementsByTagName("script"); + if (scripts.length) { + var i = scripts.length - 1; + while (i > -1 && (!scriptUrl || !/^http(s?):/.test(scriptUrl))) { + scriptUrl = scripts[i--].src; + } + } + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (document.currentScript && document.currentScript.tagName.toUpperCase() === 'SCRIPT') { scriptUrl = document.currentScript.src; } if (!scriptUrl) { var scripts = document.getElementsByTagName("script"); if (scripts.length) { var i = scripts.length - 1; while (i > -1 && (!scriptUrl || !/^http(s?):/.test(scriptUrl))) { scriptUrl = scripts[i--].src; } } }crates/mako/src/config.rs (1)
309-313:
⚠️ Potential issue修正
public_path验证逻辑中的错误当前的验证条件存在逻辑错误，
any(|p| *p != config.public_path)将始终返回true，因为config.public_path不可能同时等于"runtime"和"auto"。这会导致无法正确验证public_path的合法性。建议修改验证逻辑为：
if ["runtime", "auto"].iter().any(|p| *p != config.public_path) && !config.public_path.ends_with('/') { - return Err(anyhow!("public_path must end with '/' or be 'runtime'")); + return Err(anyhow!("public_path must end with '/' or be 'runtime' or 'auto'")); }或者更正条件判断：
- if ["runtime", "auto"].iter().any(|p| *p != config.public_path) + if config.public_path != "runtime" && config.public_path != "auto" && !config.public_path.ends_with('/') { return Err(anyhow!("public_path must end with '/' or be 'runtime' or 'auto'")); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if config.public_path != "runtime" && config.public_path != "auto" && !config.public_path.ends_with('/') { return Err(anyhow!("public_path must end with '/' or be 'runtime' or 'auto'")); }
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
docs/config.zh-CN.md (1)
630-632: 建议改进列表格式为保持文档格式一致性，建议将星号列表改为破折号列表。
-* `"runtime"`，这意味着它将切换到运行时模式并使用运行时的 `window.publicPath` 作为 publicPath; -* `"auto"`，类似 webpack 的 `publicPath: "auto"`。 +- `"runtime"`，这意味着它将切换到运行时模式并使用运行时的 `window.publicPath` 作为 publicPath; +- `"auto"`，类似 webpack 的 `publicPath: "auto"`。🧰 Tools
🪛 Markdownlint (0.35.0)
631-631: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
632-632: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
docs/config.md (1)
632-636: 建议改进英文语法和格式文档中存在一些语法问题，建议进行以下修改：
-publicPath configuration. Note: There is two special values +publicPath configuration. Note: There are two special values -- `"runtime"`, which means that it will switch to runtime mode and use the runtime `window.publicPath` as publicPath. -- `"auto"`, which is just like `publicPath: "auto"` in webpack +- `"runtime"`, which means that it will switch to runtime mode and use the runtime `window.publicPath` as publicPath. +- `"auto"`, which is just like `publicPath: "auto"` in webpack🧰 Tools
🪛 LanguageTool
[grammar] ~632-~632: Did you mean “There are two special values”?
Context: ..."/"publicPath configuration. Note: There is two special values -"runtime", which means that it wil...(THERE_S_MANY)
[uncategorized] ~634-~634: Loose punctuation mark.
Context: ...ere is two special values -"runtime", which means that it will switch to runt...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~636-~636: Loose punctuation mark.
Context: ...w.publicPathas publicPath. -"auto", which is just likepublicPath: "auto"`...(UNLIKELY_OPENING_PUNCTUATION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
crates/mako/src/config.rs(1 hunks)crates/mako/src/plugins/runtime.rs(1 hunks)docs/config.md(1 hunks)docs/config.zh-CN.md(1 hunks)e2e/fixtures/config.public_path.auto/expect.js(1 hunks)e2e/fixtures/config.public_path.auto/mako.config.json(1 hunks)e2e/fixtures/config.public_path.auto/src/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- e2e/fixtures/config.public_path.auto/src/index.tsx
- e2e/fixtures/config.public_path.auto/mako.config.json
- e2e/fixtures/config.public_path.auto/expect.js
- crates/mako/src/config.rs
🧰 Additional context used
🪛 LanguageTool
docs/config.md
[grammar] ~632-~632: Did you mean “There are two special values”?
Context: ... "/" publicPath configuration. Note: There is two special values - "runtime", which means that it wil...
(THERE_S_MANY)
[uncategorized] ~634-~634: Loose punctuation mark.
Context: ...ere is two special values - "runtime", which means that it will switch to runt...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~636-~636: Loose punctuation mark.
Context: ...w.publicPathas publicPath. -"auto", which is just like publicPath: "auto"`...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 Markdownlint (0.35.0)
docs/config.zh-CN.md
631-631: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
632-632: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
🔇 Additional comments (2)
crates/mako/src/plugins/runtime.rs (2)
29-35: 运行时模式的实现看起来不错！
运行时模式的实现正确地使用了全局上下文来设置 publicPath。
60-68: 默认模式的实现看起来不错！
默认情况下的 publicPath 设置实现正确。
crates/mako/src/plugins/runtime.rs
Outdated
| "auto" => { | ||
| r#"/* mako/runtime/publicPath */ | ||
| !function() {{ | ||
| var scriptUrl; | ||
| if (!self.document && self.importScripts) { | ||
| scriptUrl = window.location + ""; | ||
| } | ||
| if (!scriptUrl && document) { | ||
| if (document.currentScript && document.currentScript.tagName.toUpperCase() === 'SCRIPT') | ||
| scriptUrl = document.currentScript.src; | ||
| if (!scriptUrl) { | ||
| var scripts = document.getElementsByTagName("script"); | ||
| if(scripts.length) { | ||
| var i = scripts.length - 1; | ||
| while (i > -1 && (!scriptUrl || !/^http(s?):/.test(scriptUrl))) scriptUrl = scripts[i--].src; | ||
| } | ||
| } | ||
| } | ||
| if (!scriptUrl) throw new Error("Automatic publicPath is not supported in this browser"); | ||
| scriptUrl = scriptUrl.replace(/#.*$/, "").replace(/\?.*$/, "").replace(/\/[^\/]+$/, "/"); | ||
| requireModule.publicPath = scriptUrl; | ||
| }}();"# | ||
| } | ||
| .to_string(), |
There was a problem hiding this comment.
💡 Codebase verification
建议修复 Web Worker 环境中的兼容性问题
在 Web Worker 环境中，window.location 的访问确实存在问题，需要修改为 self.location。但关于 document.currentScript 的兼容性问题，经过验证：
- 当前代码已经包含了降级处理逻辑，在
document.currentScript不可用时会回退到遍历所有 script 标签的方案 - e2e 测试用例也验证了这个逻辑的正确性
建议修改：
if (!self.document && self.importScripts) {
- scriptUrl = window.location + "";
+ scriptUrl = self.location + "";
}🔗 Analysis chain
建议增强浏览器兼容性处理
当前的实现在某些场景下可能存在兼容性问题：
- Web Worker 环境中的
window.location访问 - 较老浏览器中
document.currentScript的支持
建议添加以下增强：
if (!self.document && self.importScripts) {
- scriptUrl = window.location + "";
+ scriptUrl = self.location + "";
}运行以下脚本来验证 document.currentScript 的浏览器支持情况：
Let's check the content of the expect.js file to see if it contains any compatibility handling for document.currentScript.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查 codebase 中是否有其他地方处理了 document.currentScript 的兼容性
rg -l "document.currentScript"
Length of output: 112
Script:
#!/bin/bash
# Check the content of expect.js for document.currentScript handling
cat e2e/fixtures/config.public_path.auto/expect.js
Length of output: 391
xusd320
force-pushed
the
feat/publicPath-auto
branch
from
7ec1f59 to
ce9ad87
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1717 +/- ##
=======================================
Coverage 54.91% 54.92%
=======================================
Files 178 178
Lines 17840 17843 +3
=======================================
+ Hits 9797 9800 +3
Misses 8043 8043 ☔ View full report in Codecov by Sentry. |
See: https://webpack.docschina.org/guides/public-path/#Automatic-publicPath-automaticpublicPath
Summary by CodeRabbit
publicPath配置，新增了特殊值"runtime"和"auto"。mako.config.json，设置publicPath为"auto"。publicPath、resolve和px2rem配置的说明。expect.js，用于验证requireModule.publicPath的正确性。src/index.tsx中添加了控制台日志输出。