Skip to content

feat(minifier): substitute function.call(this) for iife#16619

Draft
armano2 wants to merge 8 commits intooxc-project:mainfrom
armano2:feat/substitute_function_call_bind_this_for_iife
Draft

feat(minifier): substitute function.call(this) for iife#16619
armano2 wants to merge 8 commits intooxc-project:mainfrom
armano2:feat/substitute_function_call_bind_this_for_iife

Conversation

@armano2
Copy link
Copy Markdown
Contributor

@armano2 armano2 commented Dec 9, 2025

there is a need for an asusmption here that user has not modified Function.prototype.call to do something else

Function.prototype.call = (arg) => { console.log(arg); }
(function () {}).call("???") 

@github-actions github-actions Bot added A-minifier Area - Minifier A-ast Area - AST C-enhancement Category - New feature or request labels Dec 9, 2025
@armano2 armano2 marked this pull request as ready for review December 11, 2025 19:17
Comment thread crates/oxc_minifier/src/peephole/minimize_functions.rs
@Boshen Boshen requested a review from sapphi-red December 12, 2025 05:35
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Dec 12, 2025

Merging this PR will not alter performance

✅ 44 untouched benchmarks
⏩ 12 skipped benchmarks1


Comparing armano2:feat/substitute_function_call_bind_this_for_iife (355a57e) with main (1f57448)

Open in CodSpeed

Footnotes

  1. 12 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

test("(function* test () {foo()}).call(this)", "(function* () {foo()}).call(this)");
test("(function* test () {}).call(this)", "(function* () {}).call(this)");
test("(async function test(){foo()}).call(this)", "(async function (){foo()}).call(this)");
test_same("(function test() {console.log(test.name)}).call(this)");
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.

Suggested change
test_same("(function test() {console.log(test.name)}).call(this)");
test_same("(function test() {console.log(test.name)}).call(this)");
test_same("(function () {console.log(arguments)}).call(this)");
test_same("(function () {console.log(new.target)}).call(this)");

We need to handle these two cases.

There's also this behavior difference. But I didn't find a way to trigger it with IIFEs.

The thisArg value is passed without modification as the this value. This is a change from Edition 3, where an undefined or null thisArg is replaced with the global object and ToObject is applied to all other values and that result is passed as the this value. Even though the thisArg is passed without modification, non-strict functions still perform these transformations upon entry to the function.
https://tc39.es/ecma262/2025/multipage/fundamental-objects.html#sec-function.prototype.call

Copy link
Copy Markdown
Contributor Author

@armano2 armano2 Dec 13, 2025

Choose a reason for hiding this comment

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

this is actually going to be hard, i have hard time finding a way to check all nodes outside of adding impl for Visit,

i'm actually surprised that scope don't contain more information's

either by setting a bitmask flag or using references like state,

  • ThisExpression -> this
  • Identifier -> arguments ()
  • Super -> super
  • MetaProperty -> new.target

q, and toughts?

iterating over entire tree is not optimal, eg, loadsh wraps entire codebase in iff like call, and if user imports any iffe package, this can degrade performance just to remove few bytes

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.

I don't have an idea here. @Boshen What do you think?

Copy link
Copy Markdown
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

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

See the comment above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast Area - AST A-minifier Area - Minifier C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants