Skip to content

fix(mql): load ky lazily so CommonJS works without require(esm) - #74

Merged
Kikobeats merged 2 commits into
masterfrom
Kikobeats/mql-lazy-ky
Oct 1, 2026
Merged

Kikobeats merged 2 commits into
masterfrom
Kikobeats/mql-lazy-ky

Conversation

@Kikobeats

@Kikobeats Kikobeats commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

Problem

packages/mql/src/index.js did a top-level require('ky'), and ky@2 is ESM-only. Runtimes that reject require() of ES modules crash with ERR_REQUIRE_ESM as 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).

Error [ERR_REQUIRE_ESM]: require() of ES Module /var/task/node_modules/ky/distribution/index.js
from /var/task/node_modules/@microlink/mql/src/index.js not supported.

Fix

  • ky is loaded with await 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: true on the single-file ESM and UMD builds, so dist/ keeps ky inlined with no extra chunk.

How it was tested

  • New regression test in test/build.mjs: requires @microlink/mql with --no-experimental-require-module and makes a real request to a local server. It fails with ERR_REQUIRE_ESM before the fix and passes after.
  • packages/mql suite: 33 tests pass, plus tsd.
  • microlink.io (packages/core) loads and makes a live metadata() call with require(esm) disabled.
  • og's server, pointed at this patched mql, returns 200 PNGs in query and URL mode with require(esm) disabled.
  • A cold review checked concurrent first calls, failed imports, error mapping (MicrolinkError still wraps HTTPError), and the UMD bundle in a browser-like sandbox. No defects found.

One small behavior change: mql.stream now reports invalid ky options as a rejected promise instead of a synchronous throw.

To ship

Publish @microlink/mql, then microlink.io, since packages/core pins @microlink/mql exactly. Then bump microlink.io in 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 stream edge cases need confidence in bundled and serverless deployments.

Overview
Fixes CommonJS load failures (ERR_REQUIRE_ESM) when @microlink/mql is require()'d in environments that block require() of ESM-only ky@2 (e.g. Vercel serverless).

ky is now loaded lazily on first use via await import('ky') with a memoized instance; doFetch and mql.stream await that helper. Failed imports are not cached so later calls can retry. mql.stream now 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 for packages/function) set inlineDynamicImports: true so single-file dist/ bundles still inline ky without a separate chunk.

A new test/build.mjs case 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

  • Bug Fixes
    • Improved CommonJS compatibility, allowing the package to be required in Node.js without enabling experimental module-loading support.
    • Requests continue to use the existing headers and retry behavior.

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>
@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

You'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.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 3239e862-9756-4331-aace-39248fe308dd

📥 Commits

Reviewing files that changed from the base of the PR and between a7ba5f9 and 440ea8e.

📒 Files selected for processing (1)
  • packages/function/rollup.config.js

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: ef8872c2-e0b7-468d-bbae-a0dcaa6bbef5

📥 Commits

Reviewing files that changed from the base of the PR and between f4eb879 and a7ba5f9.

📒 Files selected for processing (3)
  • packages/mql/rollup.config.js
  • packages/mql/src/index.js
  • packages/mql/test/build.mjs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The MQL package now loads and configures ky on first request, and both Rollup outputs inline dynamic imports. A CommonJS integration test runs without experimental require(esm) support and checks a JSON response.

Changes

MQL CommonJS loading

Layer / File(s) Summary
Runtime loading and build compatibility
packages/mql/src/index.js, packages/mql/rollup.config.js, packages/mql/test/build.mjs
A cached async getKy helper dynamically imports and configures ky. doFetch and mql.stream await the helper. Both Rollup outputs inline dynamic imports. The integration test calls the CommonJS package against a local JSON endpoint without experimental require(esm) support.

Priority: ⬆️ High

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to a7ba5

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 Summary

Architecture risk: 🟡 Medium · up to a7ba5

The change affects 1 system.

Changed systems: packages/mql

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — packages/mql (library) was modified; 3 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in packages/mql/rollup.config.js: The ES output now inlines dynamic imports.
  • observed — Modified behavior in packages/mql/rollup.config.js: The UMD output now inlines dynamic imports; its name, file, and format settings are unchanged.
  • observed — Modified behavior in packages/mql/src/index.js: The static ky import was removed; ky is now loaded dynamically by getKy.
  • observed — Modified behavior in packages/mql/src/index.js: The eagerly initialized kyInstance was replaced by a cached async getKy helper. On its first call, it dynamically imports ky, applies the existing user-agent header and retry status-code settings, and stores the configured instance; later calls return the cached instance.

Reliability and maintainability

  • inferred — Risk-relevant change factors for packages/mql: blast_radius_2; direct_dependents_2
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: lazy loading of ky to support CommonJS environments without require(esm).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@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>
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 36878017029

Warning

No base build found for commit f4eb879 on master.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 81.486%

Details

  • Patch coverage: 3 uncovered changes across 1 file (17 of 20 lines covered, 85.0%).

Uncovered Changes

File Changed Covered %
packages/mql/src/index.js 20 17 85.0%

Coverage Regressions

Requires a base build to compare against. How to fix this →


Coverage Stats

Coverage Status
Relevant Lines: 5935
Covered Lines: 4872
Line Coverage: 82.09%
Relevant Branches: 1038
Covered Branches: 810
Branch Coverage: 78.03%
Branches in Coverage %: Yes
Coverage Strength: 28.54 hits per line

💛 - Coveralls

@Kikobeats
Kikobeats merged commit 6cc7977 into master Oct 1, 2026
9 checks passed
@Kikobeats
Kikobeats deleted the Kikobeats/mql-lazy-ky branch October 1, 2026 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants