fix(mql): load ky lazily so CommonJS works without require(esm) - #74
Conversation
ky@2 is ESM-only. The top-level require('ky') crashed every CommonJS
consumer in runtimes that reject require() of ES modules, such as
Vercel's serverless loader (ERR_REQUIRE_ESM on load). This took down
og.microlink.io, which uses mql through microlink.io.
ky is now loaded with import() on the first request and memoized; a
failed import is not cached, so the next call retries. Requests were
already async, so the API is unchanged. The single-file rollup builds
inline the dynamic import, so the dist bundles stay self-contained.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 55 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe MQL package now loads and configures ChangesMQL CommonJS loading
Priority: ⬆️ High Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to This change loads the HTTP client on first use so CommonJS consumers no longer hit a require-of-ESM error. No actionable merge-blocking risk remains. Architecture SummaryArchitecture risk: 🟡 Medium · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
Reliability and maintainability
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@microlink/function bundles mql's source into a single dist/index.js. mql now loads ky with import(), which rollup splits into a second chunk and rejects for single-file output (validateOptionsForMultiChunkOutput). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Coverage Report for CI Build 36878017029Warning No base build found for commit Coverage: 81.486%Details
Uncovered Changes
Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
Problem
packages/mql/src/index.jsdid a top-levelrequire('ky'), andky@2is ESM-only. Runtimes that rejectrequire()of ES modules crash withERR_REQUIRE_ESMas soon as the module loads. Vercel's serverless loader is one of them, even on Node 24.This took down og.microlink.io: every route returns 500 since og started using
microlink.io(microlinkhq/og#43).Fix
kyis loaded withawait import('ky')on the first request and memoized. A failed import is not cached, so the next call retries. Requests were already async, so the public API is unchanged.rollup.config.js:inlineDynamicImports: trueon the single-file ESM and UMD builds, sodist/keepskyinlined with no extra chunk.How it was tested
test/build.mjs: requires@microlink/mqlwith--no-experimental-require-moduleand makes a real request to a local server. It fails withERR_REQUIRE_ESMbefore the fix and passes after.packages/mqlsuite: 33 tests pass, plustsd.microlink.io(packages/core) loads and makes a livemetadata()call withrequire(esm)disabled.mql, returns 200 PNGs in query and URL mode withrequire(esm)disabled.MicrolinkErrorstill wrapsHTTPError), and the UMD bundle in a browser-like sandbox. No defects found.One small behavior change:
mql.streamnow reports invalid ky options as a rejected promise instead of a synchronous throw.To ship
Publish
@microlink/mql, thenmicrolink.io, sincepackages/corepins@microlink/mqlexactly. Then bumpmicrolink.ioin og.🤖 Generated with Claude Code
Note
Medium Risk
Changes how HTTP requests initialize in a widely used client library; behavior is mostly equivalent but lazy import and async
streamedge cases need confidence in bundled and serverless deployments.Overview
Fixes CommonJS load failures (
ERR_REQUIRE_ESM) when@microlink/mqlisrequire()'d in environments that blockrequire()of ESM-onlyky@2(e.g. Vercel serverless).kyis now loaded lazily on first use viaawait import('ky')with a memoized instance;doFetchandmql.streamawait that helper. Failed imports are not cached so later calls can retry.mql.streamnow surfaces invalid options as a rejected promise instead of a sync throw.Rollup ES and UMD outputs for
@microlink/mql(and the same ES setting forpackages/function) setinlineDynamicImports: trueso single-filedist/bundles still inlinekywithout a separate chunk.A new
test/build.mjscase runs Node with--no-experimental-require-module, requires the package, and performs a real fetch against a local HTTP server.Reviewed by Cursor Bugbot for commit 440ea8e. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit