feat: support cjs and esm both by tshy #5
Conversation
BREAKING CHANGE: drop Node.js < 18.19.0 support part of eggjs/egg#3644 eggjs/egg#5257
WalkthroughThe pull request introduces a comprehensive modernization of the project's configuration and development infrastructure. It replaces legacy configuration files like Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant CI as GitHub Actions
participant NPM as NPM Registry
participant Codecov as Codecov
Dev->>CI: Push code
CI->>CI: Run ESLint
CI->>CI: Run TypeScript tests
CI->>Codecov: Upload coverage report
CI->>NPM: Publish package
Codecov->>Dev: Provide coverage insights
Poem
Finishing Touches
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 (
|
commit: |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
![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
🧹 Nitpick comments (5)
src/index.ts (1)
10-10: Consider adding input validation for negative limit values.While the nullish coalescing operator is a good improvement for handling the limit option, the function should validate that the limit is not negative.
- const limit = options.limit ?? 512; + const limit = Math.max(0, options.limit ?? 512);Also applies to: 16-16
test/index.test.ts (1)
54-57: Add more test cases for limit validation.Consider adding test cases for:
- Zero limit
- Negative limit
- Very large limit values
package.json (1)
26-38: Consider pinning dependency versions.The devDependencies use major version ranges which could lead to unexpected breaking changes. Consider using exact versions or more restrictive ranges.
Example:
- "@arethetypeswrong/cli": "^0.17.1", + "@arethetypeswrong/cli": "0.17.1",README.md (2)
20-20: Fix grammar in the description.Change "other web framework" to "other web frameworks" for correct plural form.
-Helper to create more safe jsonp response body for [koa](http://koajs.com/) and other web framework. +Helper to create more safe jsonp response body for [koa](http://koajs.com/) and other web frameworks.🧰 Tools
🪛 LanguageTool
[uncategorized] ~20-~20: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ... koa and other web framework. ## Install ```bash npm install jsonp...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
Line range hint
48-56: Consider adding TypeScript type information.Since the project now supports TypeScript, consider adding type information to the API documentation.
Example addition:
interface JsonpOptions { limit?: number; replacer?: (key: string, value: any) => any; space?: string | number; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
.eslintignore(1 hunks).eslintrc(1 hunks).github/workflows/nodejs.yml(1 hunks).github/workflows/pkg.pr.new.yml(1 hunks).github/workflows/release.yml(1 hunks).gitignore(1 hunks).jshintignore(0 hunks).jshintrc(0 hunks).npmignore(0 hunks).travis.yml(0 hunks)AUTHORS(0 hunks)README.md(2 hunks)index.d.ts(0 hunks)package.json(2 hunks)src/index.ts(2 hunks)test/index.test.js(0 hunks)test/index.test.ts(1 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (7)
- AUTHORS
- .travis.yml
- .jshintignore
- .npmignore
- .jshintrc
- index.d.ts
- test/index.test.js
✅ Files skipped from review due to trivial changes (3)
- .eslintrc
- tsconfig.json
- .eslintignore
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~20-~20: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ... koa and other web framework. ## Install ```bash npm install jsonp...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
🪛 Biome (1.9.4)
test/index.test.ts
[error] 46-46: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Node.js / Test (windows-latest, 22)
- GitHub Check: Node.js / Test (windows-latest, 20)
- GitHub Check: Node.js / Test (windows-latest, 18.19.0)
🔇 Additional comments (9)
src/index.ts (2)
1-8: LGTM! Well-structured Options interface with clear documentation.The interface makes good use of TypeScript's utility types for JSON.stringify parameters and includes helpful JSDoc comments.
20-22: LGTM! Proper handling of line terminators.Good security practice to escape the line terminator characters \u2028 and \u2029.
.gitignore (1)
1-11: LGTM! Comprehensive .gitignore for modern Node.js development.The file properly excludes build artifacts, dependencies, and development tools while maintaining a clean structure.
.github/workflows/release.yml (1)
1-13: LGTM! Well-structured release workflow.The workflow properly reuses shared actions and handles secrets correctly. Just ensure that both NPM_TOKEN and GIT_TOKEN secrets are configured in the repository settings.
.github/workflows/nodejs.yml (1)
1-16: LGTM! Node.js versions align with the new requirements.The workflow configuration correctly tests against Node.js 18.19.0 (minimum supported version), 20, and 22, which aligns with dropping support for older versions.
.github/workflows/pkg.pr.new.yml (1)
1-23: LGTM! Verify package publishing permissions.The workflow configuration is well-structured for building and publishing. However, ensure that the
pkg-pr-new publishcommand has the necessary permissions to publish packages.Run this script to verify the package publishing configuration:
package.json (2)
22-24: LGTM! Node.js version requirement aligns with PR objectives.The engine requirement
">= 18.19.0"correctly implements the breaking change mentioned in the PR objectives.
49-75: LGTM! Excellent dual module support configuration.The configuration properly supports both ESM and CJS:
- Uses
type: "module"to enable ESM by default- Configures
exportsfield for dual package hazard prevention- Uses
tshyfor TypeScript build configuration- Correctly specifies entry points for different module systems
README.md (1)
Line range hint
31-43: LGTM! Modern JavaScript syntax in examples.The usage example has been updated to use:
- Modern
constdeclarations- ES module-style destructuring
- Async/await syntax
🧰 Tools
🪛 LanguageTool
[style] ~3-~3: Using many exclamation marks might seem excessive (in this case: 5 exclamation marks for a text that’s 933 characters long)
Context: ...y [![NPM version][npm-image]][npm-url][![Test coverage][codecov-image]][codecov-url] [![Known Vulnerabilities][snyk-image]][snyk-url] [![npm download][download-image]][download-url] [
[uncategorized] ~20-~20: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ... koa and other web framework. ## Install ```bash npm install jsonp...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[skip ci] ## [2.0.0](v1.1.0...v2.0.0) (2025-01-11) ### ⚠ 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 - **Configuration Updates** - Updated ESLint configuration to improve code quality and consistency - Removed legacy configuration files (JSHint, Travis CI) - Added TypeScript configuration with strict type checking - **CI/CD Improvements** - Replaced Travis CI with GitHub Actions workflows - Added automated testing and release processes - Configured Node.js version support (18.19.0, 20, 22) - **Project Maintenance** - Updated README with modern badges and documentation - Simplified package configuration - Removed outdated contributor information - **Development Environment** - Updated `.gitignore` to reflect current project structure - Added coverage and build-related ignore rules <!-- end of auto-generated comment: release notes by coderabbit.ai --> ### Features * support cjs and esm both by tshy ([#5](#5)) ([fa2f0ff](fa2f0ff))
BREAKING CHANGE: drop Node.js < 18.19.0 support
part of eggjs/egg#3644
eggjs/egg#5257
Summary by CodeRabbit
Configuration Updates
CI/CD Improvements
Project Maintenance
Development Environment
.gitignoreto reflect current project structure