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: use @eggjs/multipart and @eggjs/view #5391

Merged
merged 4 commits into from
Feb 4, 2025
Merged

fix: use @eggjs/multipart and @eggjs/view #5391

merged 4 commits into from
Feb 4, 2025

Conversation

fengmk2
Copy link
Member

@fengmk2 fengmk2 commented Feb 3, 2025

Summary by CodeRabbit

  • Chores

    • Switched to updated multipart and view handling packages to enhance performance and consistency.
  • Documentation

    • Clarified file upload sections with expanded guidance on both File and Stream modes.
    • Updated plugin and migration documentation with corrected links and examples to better reflect modern async standards.
    • Added directory structure details for the egg-view-ejs plugin.
    • Corrected URLs in various documentation sections to point to the new repository locations for plugins.
    • Enhanced migration guidelines to include detailed steps for transitioning to async/await syntax.

Copy link

coderabbitai bot commented Feb 3, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

The changes remove and update references to the multipart package across the project. The old dependency and import for egg-multipart have been removed, and the new dependency @eggjs/multipart has been introduced in both configuration and type definitions. Documentation has been updated accordingly, with corrected URLs, improved grammar, and expanded file upload guides. Migration guides and tests have also been revised to support these updates.

Changes

Files Change Summary
index-old.d.ts, package.json Removed import of egg-multipart and updated dependency from "egg-multipart": "^3.5.0" to "@eggjs/multipart": "^4.0.0".
src/config/plugin.ts Updated the multipart plugin configuration to reference @eggjs/multipart instead of egg-multipart.
src/app/extend/request.ts, src/lib/types.ts Updated the request body documentation to reference @eggjs/multipart and added a new PowerPartial<T> type along with a new import for @eggjs/multipart.
site/docs/basics/... Revised Controller and Plugin documentation with URL updates (switching to @eggjs/multipart), grammar corrections, and additional file upload guidelines in the Chinese version.
site/docs/intro/... Updated migration guides to revise URLs, async/await patterns, role definition examples, and other migration instructions.
test/index.test-d.ts Introduced new type checks for multipart configuration properties to ensure proper type validation.

Possibly related issues

  • None identified.

Possibly related PRs

  • fix: add createHttpClient back to app instance #5383: The changes in the main PR, which involve the removal of the index-old.d.ts file and updates to type definitions related to multipart handling, are related to the retrieved PR as both involve modifications to the Request interface and its type definitions in the context of the Egg.js framework.
  • fix: use @eggjs/onerror #5389: The changes in the main PR, which involve the removal of the index-old.d.ts file and updates to type definitions related to egg-multipart and egg-view, are related to the retrieved PR that also modifies the index-old.d.ts file by removing an import statement for egg-onerror, indicating both PRs are focused on updating type definitions and dependencies in the same file.
  • fix: mv single to @eggjs/core #5387: The changes in the main PR, which involve the removal of the index-old.d.ts file and updates to type definitions related to multipart handling, are related to the modifications in the retrieved PR that update the package references for multipart and view plugins, indicating a shift in how these functionalities are managed.

Poem

I'm a little rabbit, hopping through the code,
Finding changes hidden on every node.
From egg-multipart I’ve hopped away,
Now @eggjs/multipart leads the way.
With a skip and a jump, our code's full of cheer 🐇,
Celebrating updates that make our project clear!
Hop, hop, hooray!


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ad43946 and 1ccd59e.

📒 Files selected for processing (1)
  • index-old.d.ts (0 hunks)

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Feb 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.89%. Comparing base (351a022) to head (1ccd59e).
Report is 3 commits behind head on next.

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #5391      +/-   ##
==========================================
- Coverage   94.95%   94.89%   -0.06%     
==========================================
  Files          40       40              
  Lines        3746     3762      +16     
  Branches      355      352       -3     
==========================================
+ Hits         3557     3570      +13     
- Misses        189      192       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fengmk2 fengmk2 changed the title fix: use @eggjs/multipart fix: use @eggjs/multipart and @eggjs/view Feb 3, 2025
Copy link

socket-security bot commented Feb 3, 2025

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@eggjs/[email protected] None 0 137 kB fengmk2

View full report↗︎

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
test/lib/core/view.test.ts (1)

21-26: LGTM! Consider adding a clarifying comment.

The async implementation aligns with the transition to @eggjs/view.

Consider adding a comment to clarify that this is a mock implementation:

 class View {
+  // Mock implementation for view engine registration test
   async render() {
     return '';
   }
   async renderString() {
     return '';
   }
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3addb1c and ad43946.

📒 Files selected for processing (18)
  • index-old.d.ts (0 hunks)
  • package.json (1 hunks)
  • site/docs/advanced/view-plugin.zh-CN.md (1 hunks)
  • site/docs/basics/controller.md (2 hunks)
  • site/docs/basics/controller.zh-CN.md (2 hunks)
  • site/docs/basics/plugin.md (1 hunks)
  • site/docs/basics/plugin.zh-CN.md (1 hunks)
  • site/docs/core/view.md (1 hunks)
  • site/docs/intro/migration.md (1 hunks)
  • site/docs/intro/migration.zh-CN.md (1 hunks)
  • site/docs/tutorials/index.md (1 hunks)
  • site/docs/tutorials/index.zh-CN.md (1 hunks)
  • site/docs/tutorials/typescript.zh-CN.md (1 hunks)
  • src/app/extend/request.ts (1 hunks)
  • src/config/plugin.ts (2 hunks)
  • src/lib/types.ts (2 hunks)
  • test/index.test-d.ts (1 hunks)
  • test/lib/core/view.test.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • index-old.d.ts
✅ Files skipped from review due to trivial changes (4)
  • site/docs/core/view.md
  • site/docs/tutorials/typescript.zh-CN.md
  • site/docs/tutorials/index.md
  • site/docs/advanced/view-plugin.zh-CN.md
🚧 Files skipped from review as they are similar to previous changes (8)
  • src/app/extend/request.ts
  • src/config/plugin.ts
  • site/docs/basics/plugin.zh-CN.md
  • site/docs/basics/plugin.md
  • package.json
  • site/docs/intro/migration.zh-CN.md
  • site/docs/intro/migration.md
  • test/index.test-d.ts
🧰 Additional context used
🪛 LanguageTool
site/docs/basics/controller.md

[uncategorized] ~318-~318: The preposition ‘of’ seems more likely in this position.
Context: ...quiring the Submitted Files The body in the request can carry parameters as wel...

(AI_HYDRA_LEO_REPLACE_IN_OF)

⏰ Context from checks skipped due to timeout of 90000ms (20)
  • GitHub Check: Node.js / Test (windows-latest, 22)
  • GitHub Check: Node.js / Test (windows-latest, 20)
  • GitHub Check: Node.js / Test (windows-latest, 18)
  • GitHub Check: Node.js / Test (windows-latest, 20)
  • GitHub Check: Node.js / Test (ubuntu-latest, 22)
  • GitHub Check: Node.js / Test (windows-latest, 20)
  • GitHub Check: Node.js / Test (windows-latest, 18)
  • GitHub Check: Node.js / Test (ubuntu-latest, 20)
  • GitHub Check: Node.js / Test (windows-latest, 18)
  • GitHub Check: Node.js / Test (ubuntu-latest, 20)
  • GitHub Check: Node.js / Test (ubuntu-latest, 18)
  • GitHub Check: Node.js / Test (ubuntu-latest, 20)
  • GitHub Check: Node.js / Test (macos-latest, 22)
  • GitHub Check: Node.js / Test (ubuntu-latest, 18)
  • GitHub Check: Node.js / Test (macos-latest, 20)
  • GitHub Check: Node.js / Test (ubuntu-latest, 18)
  • GitHub Check: Node.js / Test (macos-latest, 18)
  • GitHub Check: Node.js / Test (ubuntu-latest, 22)
  • GitHub Check: Node.js / Test (windows-latest, 22)
  • GitHub Check: Node.js / Test (windows-latest, 20)
🔇 Additional comments (6)
site/docs/tutorials/index.zh-CN.md (1)

56-56: Updated egg-view Link URL

The updated link now correctly points to [egg-view]: https://github.com/eggjs/view, which aligns with the new dependency and documentation updates. Please verify that all other documentation references have been updated consistently.

src/lib/types.ts (2)

30-31: LGTM! New plugin imports added.

The imports for @eggjs/multipart and @eggjs/view are correctly placed with other plugin imports and follow the established naming convention.


86-98: Well-documented type definition!

The PowerPartial type is well-implemented with:

  • Clear JSDoc documentation and example
  • Recursive type definition for deep partial types
  • Useful for configuration objects like EggAppConfig
site/docs/basics/controller.zh-CN.md (2)

317-318: LGTM! Updated multipart plugin reference.

The reference has been correctly updated from egg-multipart to @eggjs/multipart, maintaining consistency with the package rename.


319-583: Excellent documentation for file uploads!

The documentation is comprehensive and well-structured:

  • Clear examples for both File and Stream modes
  • Proper handling of temporary files
  • Security considerations and configuration options
  • Best practices for file handling
site/docs/basics/controller.md (1)

318-592: LGTM! Documentation updates are consistent.

The English documentation:

  • Correctly references @eggjs/multipart
  • Maintains consistency with Chinese documentation
  • Provides clear and accurate translations
🧰 Tools
🪛 LanguageTool

[uncategorized] ~318-~318: The preposition ‘of’ seems more likely in this position.
Context: ...quiring the Submitted Files The body in the request can carry parameters as wel...

(AI_HYDRA_LEO_REPLACE_IN_OF)


[uncategorized] ~321-~321: The official spelling of this programming framework is “Node.js”.
Context: ...ileMode: If you have no ideas about Nodejs's Stream at all, theFile` mode suits ...

(NODE_JS)


[uncategorized] ~336-~336: Possible missing comma found.
Context: ...static front-end codes should look like this below: ```html <form method="POST" ...

(AI_HYDRA_LEO_MISSING_COMMA)


[uncategorized] ~433-~433: The official spelling of this programming framework is “Node.js”.
Context: ... you are very familiar with Stream in Nodejs, you can choose this way. In a controll...

(NODE_JS)


[style] ~479-~479: The contraction ‘there’re’ is uncommon in written English.
Context: ... To acquire the uploaded files easily, there're two conditions at least: - Only ONE fi...

(THERE_RE_CONTRACTION_UNCOMMON)


[grammar] ~479-~479: Possible agreement error. Did you mean “leasts”, “littles”?
Context: ...iles easily, there're two conditions at least: - Only ONE file per time. - The field...

(THERE_RE_MANY)


[typographical] ~482-~482: The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ile MUST be after the other fields in a form, otherwise you cannot get other fields after getti...

(THUS_SENTENCE)


[uncategorized] ~570-~570: The hyphen in Newly-added is redundant.
Context: ....js`, or rewrite a whole white list: - Newly-added a file extension: ```js module.exports...

(ADVERB_LY_HYPHEN_FIX)

@fengmk2 fengmk2 merged commit c464cda into next Feb 4, 2025
7 of 11 checks passed
@fengmk2 fengmk2 deleted the use-multipart branch February 4, 2025 02:07
fengmk2 pushed a commit that referenced this pull request Feb 4, 2025
[skip ci]

## [4.0.7](v4.0.6...v4.0.7) (2025-02-04)

### Bug Fixes

* use @eggjs/multipart and @eggjs/view ([#5391](#5391)) ([c464cda](c464cda))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

1 participant