feat: support cjs and esm both by tshy #19
Conversation
BREAKING CHANGE: drop Node.js < 18.19.0 support part of eggjs/egg#3644 eggjs/egg#5257
WalkthroughThe changes remove multiple legacy configuration and extension files, update ESLint settings, and streamline GitHub CI and release workflows. The package is rebranded from Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant CTX as Context (ViewContext)
participant VM as ViewManager
participant VE as View Engine
C->>CTX: Request view render(name, locals)
CTX->>CTX: Initialize lazy view instance if needed
CTX->>VM: Request appropriate view engine
VM->>VE: Invoke render/renderString method
VE-->>VM: Return rendered content
VM-->>CTX: Forward rendered result
CTX-->>C: Respond with rendered view
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
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 (
|
|
New, updated, and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/autod@3.1.2, npm/egg-bin@4.20.0, npm/egg-ci@1.19.1, npm/egg-mock@4.2.1, npm/mz-modules@1.0.0 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/lib/view_manager.ts (1)
107-122: Potential path security concern ininpath.
Substring checks may be bypassed by symlinks or path manipulation. Use a more robust method to confirm the resolved path is truly withinparent:-function inpath(parent: string, sub: string) { - return sub.indexOf(parent) > -1; +function inpath(parent: string, sub: string) { + const relative = path.relative(parent, sub); + return !relative.startsWith('..') && !path.isAbsolute(relative); }
🧹 Nitpick comments (10)
src/app/extend/context.ts (2)
21-34: Consider removing redundantreturn await.
Since nothing occurs after theawaitcall, you can omitawaitfor a slightly more concise implementation:- return await this.view.render(name, locals, options); + return this.view.render(name, locals, options);
36-45: Suggest dropping unnecessaryawait.
Similar to the recommendation inrenderView, you can removereturn awaitinrenderString.src/lib/view_manager.ts (1)
35-54: Recommend avoidingas anycasting.
Casting toanycan reduce type safety. Consider adding the necessary type definitions or refining the config type to remove the need forany.src/lib/context_view.ts (4)
37-39: Omit redundantawait.- return await this[RENDER](name, locals, options); + return this[RENDER](name, locals, options);
48-50: Suggest removing unneededawait.- return await this[RENDER_STRING](tpl, locals, options); + return this[RENDER_STRING](tpl, locals, options);
53-76: Minimizereturn awaitusage.
Consider directly returning the promise once resolved:- return await viewEngine.render(filename, this[SET_LOCALS](locals), options); + return viewEngine.render(filename, this[SET_LOCALS](locals), options);
78-88: Use optional chaining and avoidawaitoverhead.
Replaceoptions && options.viewEnginewithoptions?.viewEngineand remove unnecessaryreturn await:- let viewEngineName = options && options.viewEngine; + let viewEngineName = options?.viewEngine; - return await viewEngine.renderString(tpl, this[SET_LOCALS](locals), options); + return viewEngine.renderString(tpl, this[SET_LOCALS](locals), options);🧰 Tools
🪛 Biome (1.9.4)
[error] 79-79: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
test/fixtures/apps/multiple-view-engine/async.js (1)
4-12: Consider using async/await for consistency.While the Promise-based approach works, consider using async/await for consistency with other view engine implementations.
class AsyncView { - render(filename, locals, options) { + async render(filename, locals, options) { const ret = { filename, locals, options, type: 'async', }; - return scheduler.wait(10).then(() => ret); + await scheduler.wait(10); + return ret; } - renderString(tpl, locals, options) { + async renderString(tpl, locals, options) { const ret = { tpl, locals, options, type: 'async', }; - return scheduler.wait(10).then(() => ret); + await scheduler.wait(10); + return ret; }Also applies to: 14-22
src/config/config.default.ts (1)
41-49: Consider using path.resolve for root path.While
path.joinworks,path.resolvemight be more appropriate for absolute paths.export default (appInfo: EggAppInfo) => ({ view: { - root: path.join(appInfo.baseDir, 'app/view'), + root: path.resolve(appInfo.baseDir, 'app/view'), cache: true, defaultExtension: '.html', defaultViewEngine: '', mapping: {}, }, });test/fixtures/apps/multiple-view-engine/app/controller/view.js (1)
74-81: Simplify the async function implementation.The implementation can be more concise by directly returning the mapped result.
exports.renderStringTwice = async (ctx) => { const opt = { viewEngine: 'ejs' }; - const res = await Promise.all([ + return ctx.body = (await Promise.all([ ctx.renderString('a', {}, opt), ctx.renderString('b', {}, opt), - ]); - ctx.body = res.map(o => o.tpl).join(','); + ])).map(o => o.tpl).join(','); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
.autod.conf.js(0 hunks).eslintignore(1 hunks).eslintrc(1 hunks).eslintrc.js(0 hunks).github/PULL_REQUEST_TEMPLATE.md(0 hunks).github/workflows/nodejs.yml(1 hunks).github/workflows/release.yml(1 hunks).gitignore(1 hunks)README.md(4 hunks)app/extend/application.js(0 hunks)app/extend/context.js(0 hunks)config/config.default.js(0 hunks)config/config.local.js(0 hunks)index.d.ts(0 hunks)package.json(2 hunks)src/app/extend/application.ts(1 hunks)src/app/extend/context.ts(1 hunks)src/config/config.default.ts(1 hunks)src/config/config.local.ts(1 hunks)src/index.ts(1 hunks)src/lib/context_view.ts(3 hunks)src/lib/view_manager.ts(5 hunks)src/typings/index.d.ts(1 hunks)test/fixtures/apps/multiple-view-engine/app/controller/view.js(1 hunks)test/fixtures/apps/multiple-view-engine/async.js(3 hunks)test/fixtures/apps/multiple-view-engine/ejs.js(2 hunks)test/fixtures/apps/multiple-view-engine/nunjucks.js(2 hunks)test/fixtures/apps/options-root/app.js(1 hunks)test/fixtures/apps/options-root/app/router.js(1 hunks)test/view.test.ts(23 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (8)
- .github/PULL_REQUEST_TEMPLATE.md
- app/extend/application.js
- config/config.local.js
- .autod.conf.js
- config/config.default.js
- app/extend/context.js
- .eslintrc.js
- index.d.ts
✅ Files skipped from review due to trivial changes (6)
- src/index.ts
- src/typings/index.d.ts
- .eslintrc
- .eslintignore
- .gitignore
- tsconfig.json
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~3-~3: Using many exclamation marks might seem excessive (in this case: 6 exclamation marks for a text that’s 4305 characters long)
Context: ...(https://npmjs.org/package/@eggjs/view) 
(EN_EXCESSIVE_EXCLAMATION)
🪛 Biome (1.9.4)
src/lib/context_view.ts
[error] 79-79: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (52)
src/app/extend/context.ts (5)
1-6: Imports look good.
All imported modules and the Symbol definition appear correct and well-organized.
7-9: Extension of Context is appropriate.
Using a symbol property is an idiomatic way to keep the view instance private.
10-19: Render method logic is clear.
The function correctly sets the rendered content tothis.body.
47-56: View getter is well implemented.
Lazy-initialization of theContextViewinstance follows best practices.
59-66: Type augmentation is valid and consistent.
Merging new methods and properties into@eggjs/core'sContextinterface is done correctly.src/lib/view_manager.ts (7)
1-8: ES modules and imports are correct.
No syntax or path issues apparent.
9-11: ViewManagerConfig definition is coherent.
ExtendingOmit<ViewConfig, 'root'>and redefiningrootas an array is clear.
13-20: RenderOptions interface is well-structured.
Optional properties and extension of a plain object fits typical usage.
22-25: ViewEngine interface is correct.
Requiring asyncrenderandrenderStringensures consistent usage.
27-27: ViewEngineClass type is appropriate.
Clear indication that it constructs an object implementingViewEngine.
70-79: Registration checks look good.
Assertions safeguard against generator-based engines and duplicates.
90-104: Path resolution is clearly handled.
Caching and assertion on the final path are implemented properly.src/lib/context_view.ts (5)
1-4: ES module imports are properly organized.
No issues with standard imports or type imports.
17-22: Class field definitions look solid.
Using typed protected members ensures clarity for the rendering logic.
23-27: Constructor is straightforward.
Dependencies (ctx,app, andviewManager) are set up cleanly.
90-99: Engine initialization is solid.
Returning a new instance meets the expected interface for rendering.
104-110: Merging locals is well done.
Combining defaults,ctx.locals, and runtime locals ensures flexibility.src/config/config.local.ts (1)
1-7: LGTM! Clean TypeScript configuration.The configuration is well-structured, properly typed with
EggAppConfig, and follows TypeScript best practices. Settingcache: falseis appropriate for local development.test/fixtures/apps/options-root/app/router.js (1)
2-8: LGTM! Clean conversion to async/await.The conversion from generator functions to async/await is well-implemented. The use of
ctxparameter andawait ctx.render()follows modern JavaScript best practices.test/fixtures/apps/options-root/app.js (1)
3-13: LGTM! Clean conversion of View class methods.The conversion of
renderandrenderStringmethods from generator functions to async/await is well-implemented. The mock implementations are appropriate for testing purposes.test/fixtures/apps/multiple-view-engine/nunjucks.js (1)
1-22: LGTM! Clean conversion with improved dependency usage.The changes demonstrate good practices:
- Clean conversion of methods from generator functions to async/await
- Improved dependency management by using the built-in
node:timers/promisesinstead of externalmz-modules/sleeptest/fixtures/apps/multiple-view-engine/ejs.js (2)
4-13: LGTM! Clean transition from generator functions to async/await.The conversion from generator functions to async/await is well-implemented:
- Maintains the same functionality
- Uses modern JavaScript features
- Preserves the return value structure
Also applies to: 15-23
1-1: Verify Node.js version compatibility.The
node:timers/promisesmodule requires Node.js >= 16.0.0, which aligns with the PR's requirement of Node.js >= 18.19.0.✅ Verification successful
Node.js Version Compatibility Verified
The package.json correctly enforces Node.js >= 18.19.0, ensuring compatibility with the
node:timers/promisesmodule. This meets the minimum requirement (Node.js >= 16.0.0) for using that module.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify Node.js version compatibility in package.json rg -A 5 '"engines"' package.jsonLength of output: 157
test/fixtures/apps/multiple-view-engine/async.js (1)
1-1: LGTM! Consistent use of node:timers/promises.The import aligns with the changes made in other files.
src/app/extend/application.ts (3)
4-19: LGTM! Well-implemented lazy loading pattern.The implementation follows best practices:
- Uses Symbol for private storage
- Implements lazy loading
- Includes proper TypeScript types
21-25: LGTM! Clean interface extension.The declaration merging is well-implemented and properly typed.
1-2: Verify the .js extension in the import path.The import path includes a
.jsextension which is typically not needed in TypeScript files.src/config/config.default.ts (3)
1-2: LGTM! Clean imports.The imports follow best practices using the
node:protocol.
4-39: LGTM! Well-documented interface.The ViewConfig interface is:
- Thoroughly documented with JSDoc
- Has clear property types
- Includes default values in comments
51-56: LGTM! Clean interface extension.The declaration merging is well-implemented with clear comments.
test/view.test.ts (2)
70-92: LGTM! Good addition of generator function validation tests.These tests are essential for ensuring that the view engine methods are properly modernized to use async/await instead of generators.
1-9: LGTM! Clean TypeScript conversion.Good use of modern ES module imports and proper type annotations. The use of
node:protocol for built-in modules is a best practice..github/workflows/nodejs.yml (1)
1-17: LGTM! Good modernization of CI workflow.The changes align well with the PR objective to drop support for older Node.js versions. Testing on Node.js 18, 20, and 22 across multiple OS platforms provides good coverage.
✅ Verification successful
LGTM! Modernization confirmed.
The package.json now specifies a Node.js engine of ">= 18.19.0", which is fully compatible with the CI workflow testing Node.js 18, 20, and 22 across multiple OS platforms.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify Node.js version compatibility in package.json if [ -f "package.json" ]; then echo "Checking Node.js engine requirements..." jq .engines.node package.json fiLength of output: 159
package.json (13)
2-2: Update Package Name
The package name has been updated to@eggjs/view, which aligns with the new branding and repository naming.
4-6: Publish Config Addition
The newpublishConfigsection with"access": "public"ensures that the package will be published with the intended public access.
8-15: Enhanced EggPlugin Configuration
TheeggPluginsection now includes anexportsobject that defines separate entry points for ESM (import), CommonJS (require), and TypeScript (typescript). This clear configuration improves module resolution and supports multiple module formats.
23-32: Repository and Metadata Update
The metadata—including therepositoryURL,bugstracking URL,homepage, and author details—has been updated to reflect the new project structure. This keeps documentation and support links consistent with the rebranding.
33-35: Node.js Engine Requirement Update
The engines field now specifies"node": ">= 18.19.0", which is an intentional breaking change to drop support for older Node.js versions. Make sure that all dependent systems and documentation reflect this update.
36-40: Modernized Dependencies
The dependencies have been revamped by introducing libraries such as@eggjs/core,is-type-of, andutility. This update aligns with modern JavaScript practices and should enhance overall functionality.
41-56: Revamped DevDependencies
ThedevDependencieshave been overhauled to include up-to-date tools like TypeScript, ESLint, and improved testing/mocking libraries. Verify that these versions are fully compatible with Node.js >=18.19.0 and your CI/CD setup.
57-66: Updated Scripts and Build Process
The scripts section now introduces refined commands for linting, testing, cleaning, and pre-publish processing (usingtshyandtshy-after). Ensure that these new scripts integrate smoothly with your existing build and release workflows.
67-67: Module Type Declaration
Setting"type": "module"explicitly ensures that the package uses ES module syntax by default. Confirm that your tooling and build process are adjusted accordingly.
68-73: tshy Configuration Addition
The newtshysection with its ownexportsobject is added to streamline the build process. This configuration looks correct and should aid in managing module exports during packaging.
74-86: Dual Module Export Configuration
The dedicatedexportsfield now clearly defines separate entry points for both ESM and CommonJS, including associated type definitions. This is critical for ensuring compatibility across different environments.
87-90: Files Inclusion Update
Thefilesarray now explicitly includes thedistandsrcdirectories. This guarantees that only the necessary files are packaged and made available to end users.
91-93: Entry Points and Type Definitions
The updates to thetypes,main, andmodulefields ensure that the package correctly exposes its API for both TypeScript consumers and traditional CommonJS/ES module users.README.md (6)
1-1: Title and Branding Update
The title has been updated to@eggjs/view, which reflects the new branding and is consistent with the changes made in the package configuration.
3-10: Badge and Status Indicators Update
The badge URLs (for NPM version, quality, downloads, CI, test coverage, Node.js version, and PRs) have been updated to reflect the new package name and repository details. The inclusion of the CodeRabbit Pull Request Reviews badge also provides additional transparency.🧰 Tools
🪛 LanguageTool
[style] ~3-~3: Using many exclamation marks might seem excessive (in this case: 6 exclamation marks for a text that’s 4305 characters long)
Context: ...(https://npmjs.org/package/@eggjs/view)
[
18-21: Installation Instruction Update
The installation command has been corrected tonpm i @eggjs/view. This clear update minimizes confusion and helps ensure that new users install the correct package.
26-29: Plugin Configuration Example Updated
The sample configuration in the usage section now correctly referencespackage: '@eggjs/view'. This ensures consistency between documentation and the updated package configuration.
262-263: Configuration Reference Update
Linking to the new TypeScript configuration file (config/config.default.ts) directs developers to the most current configuration details, reflecting the migration from JavaScript to TypeScript.
270-277: License and Contributors Section Enhancement
The license link is clearly provided, and the new contributors section with a badge image promotes transparency and community collaboration. These updates improve the overall documentation quality.
[skip ci] ## [3.0.0](v2.1.4...v3.0.0) (2025-02-03) ### ⚠ BREAKING CHANGES * drop Node.js < 18.19.0 support part of eggjs/egg#3644 eggjs/egg#5257 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a streamlined release workflow for automated publishing. - Enhanced view rendering with asynchronous methods for improved performance. - **Refactor** - Modernized the codebase by migrating from generator functions to async/await and adopting ES module syntax. - Rebranded the package from "egg-view" to "@eggjs/view" with updated dependency management. - **Documentation** - Updated installation instructions and usage examples to reflect the new package name. - **Chores** - Upgraded Node.js support to version ≥ 18.19.0 and refined configuration settings. - **Bug Fixes** - Removed obsolete configuration files and streamlined project structure for better maintainability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ### Features * support cjs and esm both by tshy ([#19](#19)) ([c94425a](c94425a))
BREAKING CHANGE: drop Node.js < 18.19.0 support
part of eggjs/egg#3644
eggjs/egg#5257
Summary by CodeRabbit