Skip to content

fix: revert internal packages requiring esm loading for backward compatible support - #2712

Merged
zimeg merged 2 commits into
mainfrom
clack/revert-esm-only-deps
Aug 27, 2026
Merged

fix: revert internal packages requiring esm loading for backward compatible support#2712
zimeg merged 2 commits into
mainfrom
clack/revert-esm-only-deps

Conversation

@zimeg

@zimeg zimeg commented Aug 27, 2026

Copy link
Copy Markdown
Member

Summary

This pull request restores CommonJS-loadable dependency versions so require('@slack/web-api') and require('@slack/webhook') work on every Node.js version in the declared engines.node: ">= 20" range. Fixes #2711.

  • @slack/web-api@8.1.0 throws ERR_REQUIRE_ESM when require()d on Node 20 versions before require(esm) was unflagged (i.e. Node 20.0–20.18; unflagged by default only in 20.19+ / 22.12+). @slack/web-api@8.0.0 loads fine because it shipped with the older CommonJS deps.
  • Root cause: p-queue and p-retry each went pure ESM ("type": "module") at a major that our ranges silently crossed:
    • p-queue — CJS through 6.6.2; ESM-only from v7.0.0. We were on ^9.
    • p-retry — CJS through 4.6.2; ESM-only from v5.0.0. We were on ^8 (v8.0.0 also raised the engine floor to Node 22).
  • Fix: pin both back to their last CommonJS releases — p-queue@^6.6.2 and p-retry@^4.6.2 — matching what shipped in the working @slack/web-api@8.0.0. 6.6.2 and 4.6.2 are the terminal versions of their CJS majors, so the caret ranges resolve to exactly those releases while staying major-gated against the ESM-only successors. Matching Dependabot ignore rules are added for web-api (mirroring the existing rule for webhook) so a future bump can't silently re-break CJS loading.
  • Not a breaking change for consumers: p-queue/p-retry are internal implementation details — never re-exported. The public RetryOptions type is defined via the retry package, not p-retry, and the SDK only uses options common to these versions (AbortError, retries, factor, minTimeout, maxTimeout, randomize). Consumers on Node 20.19+/22.12+ see no change; consumers on plain Node 20 go from broken → working.

Testing

Simulate the pre-require(esm) Node.js behavior with the flag from the issue repro:

# From packages/web-api, after npm run build:
node --no-experimental-require-module test/integration/commonjs-project/index.js
# → ✅ CJS project integration test succeeded!

# And directly, mirroring the issue:
node --no-experimental-require-module -e "require('@slack/web-api')"   # loads (was ERR_REQUIRE_ESM)
node --no-experimental-require-module -e "require('@slack/webhook')"   # loads (was ERR_REQUIRE_ESM)

Requirements

@changeset-bot

changeset-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: f57d723

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@slack/web-api Patch
@slack/webhook Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.21%. Comparing base (c663dc0) to head (f57d723).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2712      +/-   ##
==========================================
+ Coverage   89.15%   89.21%   +0.05%     
==========================================
  Files          65       65              
  Lines       10393    10393              
  Branches      473      473              
==========================================
+ Hits         9266     9272       +6     
+ Misses       1096     1089       -7     
- Partials       31       32       +1     
Flag Coverage Δ
cli-hooks 89.21% <ø> (+0.05%) ⬆️
cli-test 89.21% <ø> (+0.05%) ⬆️
logger 89.21% <ø> (+0.05%) ⬆️
oauth 89.21% <ø> (+0.05%) ⬆️
socket-mode 89.21% <ø> (+0.05%) ⬆️
web-api 89.21% <ø> (+0.05%) ⬆️
webhook 89.21% <ø> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zimeg
zimeg force-pushed the clack/revert-esm-only-deps branch 2 times, most recently from 6bd4e94 to e2aadfd Compare August 27, 2026 18:37
…atible support

p-queue (ESM-only since v7) and p-retry (ESM-only since v5) ship as pure ESM
("type": "module"), which made the CommonJS builds of @slack/web-api and
@slack/webhook throw ERR_REQUIRE_ESM under require() on the pre-require(esm)
Node.js in the declared ">= 20" support range (Node 20.0-20.18). Pin both back
to their last CommonJS releases — p-queue@^6.6.2 and p-retry@^4.6.2 — matching
what shipped in the working @slack/web-api@8.0.0. These are the terminal
versions of their CJS majors, so the caret ranges resolve to exactly those
releases while staying major-gated against the ESM-only successors; matching
Dependabot ignore rules for web-api keep an accidental bump from re-breaking it.

No public API or runtime behavior change: p-queue/p-retry are internal
implementation details, RetryOptions is defined via the `retry` package (not
p-retry), and only options common to these versions are used.

Fixes #2711

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
@zimeg
zimeg force-pushed the clack/revert-esm-only-deps branch from e2aadfd to 3bd4240 Compare August 27, 2026 18:39
@zimeg zimeg added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented pkg:web-api applies to `@slack/web-api` pkg:webhook applies to `@slack/webhook` dependencies Pull requests that update a dependency file javascript Pull requests that update Javascript code semver:patch labels Aug 27, 2026
@zimeg zimeg self-assigned this Aug 27, 2026
@zimeg
zimeg marked this pull request as ready for review August 27, 2026 18:42
@zimeg
zimeg requested a review from a team as a code owner August 27, 2026 18:42

@WilliamBergamin WilliamBergamin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this 🙏

Left one none blocking comment

Comment thread .github/dependabot.yml Outdated
Comment on lines +56 to +57
"p-queue": "^6.6.2",
"p-retry": "^4.6.2",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 🙏

Condense the p-queue/p-retry ignore rationale to a single line per block,
keeping the #2711 reference inline.

Co-Authored-By: William Bergamin <25348381+WilliamBergamin@users.noreply.github.com>
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
@zimeg
zimeg merged commit aef66ac into main Aug 27, 2026
13 checks passed
@zimeg
zimeg deleted the clack/revert-esm-only-deps branch August 27, 2026 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented dependencies Pull requests that update a dependency file javascript Pull requests that update Javascript code pkg:web-api applies to `@slack/web-api` pkg:webhook applies to `@slack/webhook` semver:patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@slack/web-api 8.1.0: CJS build requires ESM-only p-queue@9 and p-retry@8, ERR_REQUIRE_ESM on Node 20

2 participants