fix: unpkg redirect #479
Merged
fix: unpkg redirect #479
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
elrrrrrrr
commented
May 29, 2023
elrrrrrrr
commented
- 🧶 Change the default logic when processing main field
- 📦 Keep operational logic consistent with unpkg ref
- 🧶 修改 main 字段默认逻辑判断口径
- 📦 操作逻辑和 unpkg 保持一致 ref
Codecov Report
@@ Coverage Diff @@
## master #479 +/- ##
=======================================
Coverage 97.39% 97.39%
=======================================
Files 168 168
Lines 15683 15684 +1
Branches 2012 2013 +1
=======================================
+ Hits 15274 15275 +1
Misses 409 409
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -109,7 +109,8 @@ export class PackageVersionFileController extends AbstractController { | |||
| } | |||
| const { manifest } = await this.packageManagerService.showPackageVersionManifest(scope, name, versionOrTag); | |||
| // GET /foo/1.0.0/files => /foo/1.0.0/files/{main} | |||
| const indexFile = manifest?.main ?? 'index.js'; | |||
| // ignore empty entry exp: @types/node@20.2.5/ | |||
| const indexFile = manifest?.main || 'index.js'; | |||
| ctx.redirect(join(ctx.path, indexFile)); | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
代码本身看起来很简单，主要实现了一个包文件版本管理的控制器类。该代码修改点只有一行，保证了不会因为 manifest 中的 main 字段为空而出现程序异常错误，并且在重定向时正确处理了 index 文件名。
然而代码中似乎存在一些数值硬编码（如 files 和 index.js），这可能会增加代码维护困难度。建议可以将这些常量定义为属性或者常量变量。另外，由于代码短小精悍并没有完整的语境和上下文，所以评估函数运行效率、接口安全性等方面还需要进一步了解。
| .expect('content-type', 'application/json; charset=utf-8'); | ||
| assert.equal(res.body.error, '[NOT_FOUND] File foo@1.0.0/index.js not found'); | ||
| }); | ||
|
|
||
| it('should list one package version files', async () => { | ||
| mock(app.config.cnpmcore, 'allowPublishNonScopePackage', true); | ||
| const pkg = await TestUtil.getFullPackage({ |
There was a problem hiding this comment.
这段代码补丁中所做的改动主要是增加了一个单元测试用例。
该用例的功能为创建一个具有空main字段和自定义描述（通过getFullPackage()函数生成）的npm包，并对其进行PUT请求。测试接着发起了一次GET请求，期望该请求能正确返回404错误和一个JSON响应，表明该文件未被找到。
在代码实现方面，建议对代码缺少的类型注解进行补充，并检查mock的使用是否合理。
fengmk2
pushed a commit
that referenced
this pull request
May 29, 2023
[skip ci] ## [3.22.3](v3.22.2...v3.22.3) (2023-05-29) ### Bug Fixes * unpkg redirect ([#479](#479)) ([c395c79](c395c79))
|
🎉 This PR is included in version 3.22.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fengmk2
reviewed
May 29, 2023
| name: 'foo', | ||
| version: '1.0.0', | ||
| versionObject: { | ||
| main: '', |
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.