Keep @directive on @this functions by cknitt · Pull Request #9262 · rescript-lang/rescript

cknitt · 2026-11-07T15:28:16Z

@this functions dropped their @directive. There were two causes:

bs_builtin_ppx left the attribute on the unsafe_to_method call instead of the function, which is where translcore reads it. The PPX now moves @directive onto the inner function.
Pjs_fn_method ignored directive, and E.method_ had no parameter for it. lam_compile now passes it through.

A directive has to be the first statement of the body, so Js_dump prints it before let self = this. This also works for async functions and when self is unused.

New test: tests/tests/src/this_directive.res, a JS snapshot covering the plain, async and unused-self cases.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Kbf3gJuq6rWZULJVY3ATmY

github-actions · 2026-11-07T15:28:38Z

Changelog

🐛 Bug fix

Keep @directive on @this functions.

Semantic Diff

2 commits · Full page

Keep @directive on @this functions

_{66d6998 · cknitt + Claude · Nov 07, 2026, 15:28 UTC}

Why

The @this rewrite left @directive on the unsafe_to_method call instead of the function, and the Pjs_fn_method path built the JS function without a directive, so it was dropped without a report. The PPX now moves @directive onto the inner function, E.method_ takes it, and the printer puts it before let self = this, since a directive must be the first statement of the body.

JS modified (@this function with a directive)

let plain = @this @directive("'use cache'") (self, x) => self + x

js

 function plain(x) {
+  'use cache';
   let self = this ;
   return self + x | 0;
 }

Fixes #9197

✨ Print the this binding of a method without a space before the semicolon

_{ac091ee · cknitt + Claude · Nov 07, 2026, 15:41 UTC · new}

The space was left over from when the printer spelled out let self = by hand; no other statement puts a space before its semicolon.

JS modified (@this function)

let even = @this (o, x) => x + o

js

-let o = this ;
+let o = this;

codecov · 2026-11-07T15:33:19Z

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.95%. Comparing base (0178eb0) to head (feffddb).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
compiler/core/lam_compile.ml	50.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #9262   +/-   ##
=======================================
  Coverage   81.94%   81.95%           
=======================================
  Files         456      456           
  Lines       65575    65616   +41     
=======================================
+ Hits        53737    53777   +40     
- Misses      11838    11839    +1

Files with missing lines	Coverage Δ
compiler/core/js_dump.ml	`93.94% <100.00%> (+0.01%)`	⬆️
compiler/core/js_exp_make.ml	`66.32% <ø> (ø)`
compiler/frontend/ast_uncurry_gen.ml	`95.23% <100.00%> (ø)`
compiler/frontend/bs_builtin_ppx.ml	`90.67% <100.00%> (+0.06%)`	⬆️
compiler/core/lam_compile.ml	`90.40% <50.00%> (+0.47%)`	⬆️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

pkg-pr-new · 2026-11-07T15:39:20Z

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript@9262

@rescript/belt

npm i https://pkg.pr.new/@rescript/belt@9262

@rescript/darwin-arm64

npm i https://pkg.pr.new/@rescript/darwin-arm64@9262

@rescript/linux-arm64

npm i https://pkg.pr.new/@rescript/linux-arm64@9262

@rescript/linux-x64

npm i https://pkg.pr.new/@rescript/linux-x64@9262

@rescript/runtime

npm i https://pkg.pr.new/@rescript/runtime@9262

@rescript/win32-x64

npm i https://pkg.pr.new/@rescript/win32-x64@9262

commit: feffddb

cknitt · 2026-11-07T15:43:14Z

@codex review

chatgpt-codex-connector · 2026-11-07T15:43:30Z

Codex Review Summary: ✅ Completed

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

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-07T15:58:30.782163Z	`ac091ee`	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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ac091eec07

ℹ️ 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".

github-actions · 2026-11-07T15:56:00Z

Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-9262

cknitt · 2026-11-07T15:56:52Z

@codex review

chatgpt-codex-connector · 2026-11-07T15:58:29Z

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: ac091eec07

ℹ️ 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".

@this

Why: The @this rewrite left @directive on the unsafe_to_method call instead of the function, and the Pjs_fn_method path built the JS function without a directive, so it was dropped without a report. The PPX now moves @directive onto the inner function, E.method_ takes it, and the printer puts it before `let self = this`, since a directive must be the first statement of the body. JS modified (@this function with a directive): Example: let plain = @this @directive("'use cache'") (self, x) => self + x Before (js): function plain(x) { let self = this ; return self + x | 0; } After (js): function plain(x) { 'use cache'; let self = this ; return self + x | 0; } Fixes #9197 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gJuq6rWZULJVY3ATmY Signed-off-by: Christoph Knittel <ck@cca.io>

@this

The space was left over from when the printer spelled out `let self =` by hand; no other statement puts a space before its semicolon. JS modified (@this function): Example: let even = @this (o, x) => x + o Before (js): let o = this ; After (js): let o = this; Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gJuq6rWZULJVY3ATmY Signed-off-by: Christoph Knittel <ck@cca.io>

github-actions · 2026-11-10T16:18:02Z

Changelog

🐛 Bug fix

Keep @directive on @this functions.

Semantic Diff

2 commits · Full page

Keep @directive on @this functions

_{804df1b · cknitt + Claude · Nov 07, 2026, 15:28 UTC}

Why

The @this rewrite left @directive on the unsafe_to_method call instead of the function, and the Pjs_fn_method path built the JS function without a directive, so it was dropped without a report. The PPX now moves @directive onto the inner function, E.method_ takes it, and the printer puts it before let self = this, since a directive must be the first statement of the body.

JS modified (@this function with a directive)

let plain = @this @directive("'use cache'") (self, x) => self + x

js

 function plain(x) {
+  'use cache';
   let self = this ;
   return self + x | 0;
 }

Fixes #9197

Print the this binding of a method without a space before the semicolon

_{feffddb · cknitt + Claude · Nov 07, 2026, 15:41 UTC}

The space was left over from when the printer spelled out let self = by hand; no other statement puts a space before its semicolon.

JS modified (@this function)

let even = @this (o, x) => x + o

js

-let o = this ;
+let o = this;

github-actions · 2026-11-10T16:23:43Z

✅ Compiler benchmarks

Allocated words

In total: 682,341,187 → 682,359,517 (±0.00%).

Generated projects: 923,206,992 → 923,212,416 (±0.00%).

None of the 265 benchmarks changed by 0.1% or more.

Instructions

In total: 9,666,073,316 → 9,666,622,469 (+0.01%).

None of the 33 benchmarks changed by 0.1% or more.

Output size

In total: 25,812,251 → 25,812,418 (±0.00%).

None of the 13 benchmarks changed by 0.1% or more.

Pull request feffddb compared with base 32116b5 in this run.

github-actions · 2026-11-10T17:01:13Z

Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-9262

cristianoc

Dot, Cristiano's assistant, reviewing at his request.

Traced @directive through the PPX-generated inner function, async transformation, Lambda/Pjs_fn_method, and JS method printing. It is emitted once before the this binding. Plain, async and unused-self snapshots cover the affected paths. No blocking findings; exact-head CI is green. I did not rerun compiler/tests locally.

chatgpt-codex-connector Bot reviewed Nov 7, 2026

View reviewed changes

Comment thread compiler/core/lam_compile.ml Resolved

cknitt marked this pull request as ready for review November 7, 2026 16:01

cknitt requested a review from cristianoc November 7, 2026 16:01

cknitt and others added 2 commits November 10, 2026 16:06

cknitt force-pushed the this-directive-9197 branch from ac091ee to feffddb Compare November 10, 2026 16:08

cristianoc approved these changes Nov 13, 2026

View reviewed changes

cknitt merged commit a72e4bf into master Nov 13, 2026
22 checks passed

cknitt deleted the this-directive-9197 branch November 13, 2026 13:45

Conversation

cknitt commented Nov 7, 2026

Uh oh!

github-actions Bot commented Nov 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog

🐛 Bug fix

Semantic Diff

Uh oh!

codecov Bot commented Nov 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pkg-pr-new Bot commented Nov 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cknitt commented Nov 7, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

github-actions Bot commented Nov 7, 2026

Uh oh!

cknitt commented Nov 7, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 7, 2026

Uh oh!

github-actions Bot commented Nov 10, 2026

Changelog

🐛 Bug fix

Semantic Diff

Uh oh!

github-actions Bot commented Nov 10, 2026

✅ Compiler benchmarks

Allocated words

Instructions

Output size

Uh oh!

github-actions Bot commented Nov 10, 2026

Uh oh!

cristianoc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Nov 7, 2026 •

edited

Loading

codecov Bot commented Nov 7, 2026 •

edited

Loading

pkg-pr-new Bot commented Nov 7, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Nov 7, 2026 •

edited

Loading