-
-
Notifications
You must be signed in to change notification settings - Fork 3
BREAKING: Make module ESM only #1211
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR converts the module from a dual CommonJS/ESM package to ESM-only, aligning with the switch to the @exodus/bytes dependency which requires Node.js 20+. The change simplifies the build process by replacing tshy with plain TypeScript compilation.
Changes:
- Removed dual-module build system (tshy) in favor of ESM-only with standard TypeScript compiler
- Updated Node.js engine requirement to >=20.11.0
- Modernized TypeScript configuration (ES2022 target, node20 module)
Reviewed changes
Copilot reviewed 4 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Updated compiler target to ES2022, module to node20, changed output directory from "lib" to "dist", and updated exclusion patterns |
| package.json | Simplified exports to ESM-only, removed tshy dependency, added Node >=20.11.0 engine requirement, changed build script from tshy to tsc |
| package-lock.json | Removed tshy and all related dependencies (chokidar, foreground-child, resolve-import, sync-content, etc.) |
| sniffer.js | Removed CommonJS compatibility shim (no longer needed) |
| sniffer.d.ts | Removed CommonJS type definitions shim (no longer needed) |
| eslint.config.js | Updated Node version setting to >=20.11.0, removed obsolete ignore patterns for tshy artifacts |
| .prettierignore | Added dist/ and docs/ directories to ignore list |
| .gitignore | Removed .tshy/ directory from ignore list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Hey @fb55, the change LGTM. From the looks of it I believe cheerio will be able to upgrade the package version to consume the changes without any code change. Though should this cause a major upgrade in cheerio too? Can we merge and release a version if it looks good to you too? |
|
Hey @fb55 , just confirming the plan is not to move https://github.com/cheeriojs/cheerio to ESM only? As that will block commonjs projects from adopting the change. |
With the switch to exodus/bytes, we require Node 20. Let's combine this breaking change with a switch to ESM only.