Skip to content

Rescript 12 - #848

Merged
Freddy03h merged 14 commits into
mainfrom
rescript-12
Sep 15, 2026
Merged

Freddy03h merged 14 commits into
mainfrom
rescript-12

Conversation

@Freddy03h

@Freddy03h Freddy03h commented Apr 25, 2026

Copy link
Copy Markdown
Member

Upgrade bindings to ReScript 12 (peer >=12.0, ESM, .res.js) and clean up APIs along the way.

Tooling / platform

  • ReScript 12 + @rescript/react 0.15
  • esmodule, suffix .res.js, new CLI scripts
  • Stdlib migration (Js.*nullable / null / Int / Float / RegExp)

Breaking API cleanups

  • Collapse most multiple externals into optional labeled args (share, loop, sendIntent, …)
  • @unboxed for Vibration, Linking.ExtraValue, Image.Source
  • Image.Source: Require / URISource / URISources (replaces from*); drop getSize_legacy
  • Packager.required is float (Metro asset id)

Type organization (additive / internal)

  • Centralize Accessibility.props / iosProps / androidProps / viewProps
  • New Layout module (point, size, rectangle, insets) reused in events / ScrollView / Image
  • Image composes View.viewPropsWithoutChildren + ios/android image props
  • Nested records for local event payloads
  • W3C PointerEvent + View.pointerEventProps (also Text / Pressable / Image via View)

Not in this PR

  • Style @obj → unboxed transforms (bigger follow-up)
  • Animated.event1/event2 left as-is (reverted unification)

@Freddy03h

Copy link
Copy Markdown
Member Author

I tested on my project without issue, and it's on production.

Do you see something else you want to change or improve?

@Freddy03h
Freddy03h marked this pull request as ready for review June 16, 2026 21:49
Comment thread src/apis/Animated.res.js
};

function timing(prim0, prim1) {
return ReactNative.Animated.timing(prim0, prim1);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this seems wrong
but not sure if it was a compiler bug

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Why it's wrong? It look the same in the .bs.js

function timing(prim0, prim1) {
  return ReactNative.Animated.timing(prim0, prim1);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is not zero cost, but it could be, if you duplicate the binding definition instead of using let.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh, I see! But it's not really a regression, we've been living with it for years now, so for me it's a non-blocker for this PR.

It's definitely something we could improve as part of a more global Animated refactoring, though!

For now, IMO, the goal should be to move forward with a 0.84 release using Rescript 12.0. We're already a bit late on the release, so we can use a later one for other Rescript updates (12.1, 12.2, 12.3?) or a breaking-change refactor.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you create an issue for it?

Comment thread src/components/ActivityIndicator.res.js
@Freddy03h

Freddy03h commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

This PR has "rescript": "~12.0.2" in devDep, and "rescript": ">=12.0.0" in peerDep.
But should we force to use a more recent version of rescript 12?

Edit: I tried locally 12.1, 12.2 and 12.3, and the generated JS output is unchanged.

@cknitt

cknitt commented Sep 5, 2026

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-05T13:57:40.185091Z 941e20c Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 941e20c646

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/apis/AccessibilityInfo.res Outdated
@Freddy03h

Copy link
Copy Markdown
Member Author

Hi @fhammerschmidt @cknitt , just following up on this PR. Is everything okay on your side, and do you think we can move forward with merging it? Thanks!

@Freddy03h
Freddy03h merged commit 3a29515 into main Sep 15, 2026
2 checks passed
@Freddy03h
Freddy03h deleted the rescript-12 branch September 15, 2026 08:31
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.

3 participants