-
Notifications
You must be signed in to change notification settings - Fork 5
Rescript 12 #15
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?
Rescript 12 #15
Conversation
WalkthroughBumps ReScript toolchain and package versions, restructures rescript.json dependency keys, updates Jest config, and upgrades CI Node matrix and cache action; package versions and scripts updated across rescript-debounce and rescript-debounce-react. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (4)
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 |
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
26-28: Replace deprecated::set-outputcommand.The
::set-outputcommand was deprecated by GitHub Actions and will eventually stop working. Use the$GITHUB_OUTPUTenvironment file instead.🔧 Proposed fix
- - name: Get yarn cache directory path - id: yarn-cache-dir-path - run: echo "::set-output name=dir::$(yarn cache dir)" + - name: Get yarn cache directory path + id: yarn-cache-dir-path + run: echo "dir=$(yarn cache dir)" >> $GITHUB_OUTPUT
🤖 Fix all issues with AI agents
In @rescript-debounce/jest.config.js:
- Around line 4-5: The config references @glennsl/rescript-jest in
transformIgnorePatterns but that package targets ReScript 11.x and may be
incompatible with ReScript 12; verify compatibility by running the test suite
after upgrading to 12.0.2 and checking package.json and node_modules for a
v12-compatible release of @glennsl/rescript-jest; if it fails, remove or stop
referencing @glennsl/rescript-jest from transformIgnorePatterns (leave only
@rescript/runtime) and replace it in devDependencies with a v12-compatible
testing adapter or fork (or update to an alternative v12-ready package), then
rerun tests and CI to confirm everything passes.
In @rescript-debounce/package.json:
- Around line 38-42: The package.json currently pins "@glennsl/rescript-jest":
"0.13.1" and "babel-jest": "30.2.0", but @glennsl/rescript-jest is incompatible
with rescript "12.0.2" and babel-jest@30 requires Node >=20; replace the
incompatible test setup by either swapping "@glennsl/rescript-jest" for a
maintained runner like "rescript-vitest" (or a rescript-jest version explicitly
compatible with rescript 12), and adjust or pin "babel-jest" to a version that
matches your Node/Babel stack (or remove babel-jest if using vitest), then
update package.json dependencies and any Jest/Vitest config to ensure
ReScript-generated .res.js files are transformed (add appropriate transform
entries or plugin) and run tests to confirm failures are resolved.
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
19-19: Consider updating other action versions for consistency.While updating
actions/cacheto v4, consider also updating other actions to their latest versions for consistency, security fixes, and performance improvements:
actions/setup-node@v1→actions/setup-node@v4actions/checkout@v2→actions/checkout@v4♻️ Proposed updates
- name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v1 + uses: actions/setup-node@v4 with: node-version: ${{ matrix.node-version }} - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v4Also applies to: 24-24
14-14: Node.js 24.x is LTS-stable; consider testing multiple versions for broader compatibility.Node.js 24.x entered LTS on October 28, 2025, and is stable for CI use. However, testing against multiple Node.js versions (e.g., 20.x, 22.x, 24.x) would provide broader compatibility assurance, unless a specific reason exists to limit testing to only the latest LTS.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
rescript-debounce-react/package-lock.jsonis excluded by!**/package-lock.jsonrescript-debounce-react/yarn.lockis excluded by!**/yarn.lock,!**/*.lockrescript-debounce/package-lock.jsonis excluded by!**/package-lock.jsonrescript-debounce/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
.github/workflows/ci.ymlrescript-debounce-react/package.jsonrescript-debounce-react/rescript.jsonrescript-debounce/HISTORY.mdrescript-debounce/jest.config.jsrescript-debounce/package.jsonrescript-debounce/rescript.json
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
rescript-debounce/HISTORY.md
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🔇 Additional comments (7)
.github/workflows/ci.yml (1)
32-32: Update to actions/cache@v4 is correct and necessary for continued functionality.The migration from v1 to v4 is mandatory. This workflow uses the standard cache parameters (path, key, restore-keys) which are fully compatible with v4. Critically, the legacy cache service will be sunset on February 1, 2025—v4 is required to use the new cache service (v2) after that date. No further changes needed.
rescript-debounce/HISTORY.md (1)
3-5: LGTM! Changelog entry is clear and follows existing format.The new version entry accurately documents the ReScript v12 upgrade. The heading level is consistent with all other version entries in this file, so the markdownlint hint can be safely ignored.
rescript-debounce-react/rescript.json (1)
6-9: LGTM! Correct ReScript v12 configuration migration.The migration from
bs-dependenciestodependenciesfollows the ReScript v12 configuration format. The dependency list remains unchanged, ensuring compatibility.rescript-debounce/rescript.json (2)
10-20: Configuration structure correctly migrated to ReScript v12.The migration from
bs-dependenciestodev-dependenciesand consolidation of test dependencies follows ReScript v12's configuration patterns.
10-11: No breaking change concern — the source code does not rely on auto-opened RescriptCore APIs and ReScript v12 integrates core bindings into the compiler by default.The code uses standard APIs (Date.now, Int.fromFloat, ref operations, setTimeout/clearTimeout) without depending on an explicit RescriptCore auto-open. The removal of @rescript/core and the -open flag is the correct approach for v12 and poses no risk to this codebase.
Likely an incorrect or invalid review comment.
rescript-debounce-react/package.json (1)
39-40: Dependency versions are valid. Both @rescript/react@0.14.0 and rescript@12.0.2 exist on npm and are stable releases.rescript-debounce/package.json (1)
20-20: The format script change is correct for ReScript 12. The-allflag was removed in v12, andrescript formatnow formats all tracked files by default without requiring additional flags.
| testPathIgnorePatterns: ["/node_modules/", "/lib/"], | ||
| transformIgnorePatterns: ["node_modules/(?!(@rescript/runtime|@glennsl/rescript-jest)/)"], |
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.
🧩 Analysis chain
🏁 Script executed:
cat rescript-debounce/jest.config.jsRepository: shakacode/rescript-debounce
Length of output: 312
🌐 Web query:
ReScript v12 @rescript/runtime package structure changes
💡 Result:
Summary of the @rescript/runtime changes in ReScript v12
- The runtime modules were extracted out of the main rescript package into a standalone @rescript/runtime npm package (the rescript package now depends on it). [1][2]
- @rescript/runtime replaces the older @rescript/std package; if you had @rescript/std, remove it. [2]
- The new runtime is part of the v12 migration (some internal API renames and cleanup occurred—e.g., JsError → JsExn and many *Exn helpers renamed to *OrThrow). See the migration guide for exact symbol changes. [2]
- The compiler/build now resolves @rescript/runtime (there’s also a mechanism to override resolution via the RESCRIPT_RUNTIME environment variable as noted in the release notes). [3]
- Distribution is ESM-first and the overall packaging changed (v12 introduced platform-specific binaries and other packaging changes that affect how runtime is installed/loaded). [1]
Sources
- ReScript v12 announcement (blog). [1]
- ReScript migration guide — “Migrate to v12” (docs). [2]
- rescript-compiler release notes (v12 RCs) — mention RESCRIPT_RUNTIME resolution. [3]
🌐 Web query:
ReScript v12 Jest configuration transformIgnorePatterns
💡 Result:
Do you have a specific error or setup (jest.config.js, package.json, and an example failing file)? I can then give the exact transformIgnorePatterns and transformer entries for ReScript v12.
🏁 Script executed:
cd rescript-debounce && git log --oneline jest.config.js | head -10Repository: shakacode/rescript-debounce
Length of output: 100
🏁 Script executed:
cd rescript-debounce && git show HEAD:jest.config.js 2>/dev/null || git show HEAD~1:jest.config.js 2>/dev/null || echo "No previous version"Repository: shakacode/rescript-debounce
Length of output: 90
🌐 Web query:
@glennsl/rescript-jest ReScript v12 compatibility
💡 Result:
Short answer: Not yet — @glennsl/rescript-jest currently targets ReScript 11.x and there’s no published release that declares compatibility with ReScript v12. See the package notes (requires ReScript 11.1.x) and the ReScript v12 migration docs for the breaking changes you’d need to account for. [1][2]
If you need next steps:
- Stay on ReScript 11 until rescript-jest is updated, or
- Fork/update the repo to adapt its API/annotations and config to v12 (rename bsconfig → rescript.json, adjust annotations, std/runtime changes, etc.). [2]
Sources:
- @glennsl/rescript-jest (npm) — required ReScript 11.1.x and changelog. [1]
- ReScript v12 migration guide / breaking changes. [2]
🏁 Script executed:
cd rescript-debounce && cat package.json | grep -A 20 '"devDependencies"'Repository: shakacode/rescript-debounce
Length of output: 256
Flag potential incompatibility with @glennsl/rescript-jest in ReScript v12 setup.
The jest.config.js correctly updates transformIgnorePatterns to include @rescript/runtime, which reflects the new package structure in ReScript v12. However, the configuration also references @glennsl/rescript-jest, which currently targets ReScript 11.x and has no published v12-compatible release. Verify that this testing library works correctly with the ReScript 12.0.2 upgrade, or update/replace it with a compatible v12 testing solution.
🤖 Prompt for AI Agents
In @rescript-debounce/jest.config.js around lines 4 - 5, The config references
@glennsl/rescript-jest in transformIgnorePatterns but that package targets
ReScript 11.x and may be incompatible with ReScript 12; verify compatibility by
running the test suite after upgrading to 12.0.2 and checking package.json and
node_modules for a v12-compatible release of @glennsl/rescript-jest; if it
fails, remove or stop referencing @glennsl/rescript-jest from
transformIgnorePatterns (leave only @rescript/runtime) and replace it in
devDependencies with a v12-compatible testing adapter or fork (or update to an
alternative v12-ready package), then rerun tests and CI to confirm everything
passes.
| "@babel/core": "7.28.5", | ||
| "@babel/preset-env": "7.28.5", | ||
| "@glennsl/rescript-jest": "0.13.1", | ||
| "babel-jest": "30.2.0", | ||
| "rescript": "12.0.2" |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify package versions exist on npm
echo "=== Verifying package versions exist ==="
npm view "@babel/core@7.28.5" 2>&1 | head -5
echo "---"
npm view "@babel/preset-env@7.28.5" 2>&1 | head -5
echo "---"
npm view "@glennsl/rescript-jest@0.13.1" 2>&1 | head -5
echo "---"
npm view "babel-jest@30.2.0" 2>&1 | head -5
echo "---"
npm view "rescript@12.0.2" 2>&1 | head -5Repository: shakacode/rescript-debounce
Length of output: 764
🌐 Web query:
rescript 12.0.2 babel-jest 30.2.0 compatibility
💡 Result:
Short answer: yes — ReScript 12.0.2 can work with babel-jest 30.2.0, but you must meet the platform and tooling requirements and configure Jest/Babel to transform ReScript output.
Key points and actions:
- ReScript 12 requires Node >= 20 (see migration/installation notes). [1][2]
- Jest / babel-jest v30.x targets Jest 30 and expects modern Node versions (Jest 30’s engine list moved to Node 18+/20 in recent releases). Install @babel/core as babel-jest’s peer dependency. [3][4]
- Configure Babel/Jest to transform ReScript-generated files (usually *.res.js). Common patterns:
- add babel.config.js (or babel preset) and install @babel/core, babel-jest;
- set Jest transform/transformIgnorePatterns to include .res.js files (or allow node_modules transforms for any rescript-published packages you test). Example pattern: transformIgnorePatterns: ["/node_modules/(?!rescript).+\.res\.js$"]. [5][6]
- If you use the new ReScript 12 ESM distribution, ensure your Jest/Babel setup supports ESM (or build to CommonJS) — check your Babel presets/targets and jest config.
If you want, tell me:
- the exact ReScript package version in your project (12.0.2 vs 12.0.x) and your Node version, and I’ll give the precise package install commands and a minimal jest + babel config that worked for ReScript output.
Sources:
[1] ReScript — Migrate to v12 / installation requirements.
[2] Announcing ReScript 12 (release notes).
[3] jest releases (v30.2.0).
[4] babel-jest / babel-preset-jest packaging/peer info.
[5] ReScript + Jest examples/gist (transformIgnorePatterns).
[6] ReScript docs (integration notes about generating .res.js that standard toolchains must transform).
🌐 Web query:
@glennsl/rescript-jest 0.13.1 rescript 12.0.2 compatibility
💡 Result:
Short answer: No—there’s no published @glennsl/rescript-jest 0.13.1 that declares compatibility with ReScript 12, and the released rescript-jest package targets much older ReScript versions, so you shouldn’t expect it to work with rescript 12.0.2 without changes.
Evidence:
- The npm package @glennsl/rescript-jest shows v0.11.0 as the published version and documents a dependency on older ReScript (example: “depends on Rescript 9.1.4”). [1]
- ReScript 12 contains multiple breaking changes and a migration guide that requires library updates (rename/config changes, new build system, runtime split, etc.), so libraries built for earlier ReScript versions commonly need updates to support v12. [2][3]
- There are community ports (e.g. a rescript-vitest ported from glennsl’s rescript-jest) indicating maintainers/consumers are moving to alternatives rather than relying on an updated rescript-jest yet. [4]
If you want, I can:
- check the GitHub repo/tags for any unreleased 0.13.x branch or PRs that add ReScript 12 support, or
- suggest migration steps or alternative test bindings (rescript-vitest) that are known to work with newer ReScript versions.
Sources:
[1] @glennsl/rescript-jest npm page (package info & version).
[2] ReScript “Migrate to v12” manual (breaking changes, migration notes).
[3] ReScript blog/forum posts about ReScript 12 (release notes / roadmap).
[4] Forum announcement of a rescript-vitest port from @glennsl/rescript-jest.
@glennsl/rescript-jest@0.13.1 is incompatible with rescript@12.0.2.
All dependency versions exist on npm. However, verification reveals a critical compatibility issue: @glennsl/rescript-jest was built for older ReScript versions and is not maintained for ReScript 12's breaking changes. This will cause test failures.
Additionally, babel-jest@30.2.0 requires Node >= 20 and proper Babel/Jest configuration to transform ReScript-generated .res.js files. Consider updating to a maintained alternative like rescript-vitest or updating rescript-jest to a compatible version before merging.
🤖 Prompt for AI Agents
In @rescript-debounce/package.json around lines 38 - 42, The package.json
currently pins "@glennsl/rescript-jest": "0.13.1" and "babel-jest": "30.2.0",
but @glennsl/rescript-jest is incompatible with rescript "12.0.2" and
babel-jest@30 requires Node >=20; replace the incompatible test setup by either
swapping "@glennsl/rescript-jest" for a maintained runner like "rescript-vitest"
(or a rescript-jest version explicitly compatible with rescript 12), and adjust
or pin "babel-jest" to a version that matches your Node/Babel stack (or remove
babel-jest if using vitest), then update package.json dependencies and any
Jest/Vitest config to ensure ReScript-generated .res.js files are transformed
(add appropriate transform entries or plugin) and run tests to confirm failures
are resolved.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.