Skip to content

Downgrade ES6+ syntax in a couple of key harness files#5006

Open
ivankra wants to merge 1 commit intotc39:mainfrom
ivankra:es5
Open

Downgrade ES6+ syntax in a couple of key harness files#5006
ivankra wants to merge 1 commit intotc39:mainfrom
ivankra:es5

Conversation

@ivankra
Copy link

@ivankra ivankra commented Mar 25, 2026

Replace template literals with equivalent string concatenation in assert.js and remove the trailing argument comma in propertyHelper.js.

This makes the harness ES5-friendlier and allows nearly a third of the tests to pass on old ES5 engines from a baseline of ~8.7% of all negative cases - assert.js is a key blocker.


Improvement in pass rate in my testing:

engine 5c82069 assert.js +propertyHelper.js
duktape 4639 (8.7%) 16675 (31.4%) 18268 (34.4%)
iv-lv5 4633 (8.7%) 15911 (30.0%) 17126 (32.2%)
kjs 4631 (8.7%) 13670 (25.7%) 14585 (27.5%)
mujs 4635 (8.7%) 13630 (25.7%) 14215 (26.8%)

Replace template literals with equivalent string concatenation in
assert.js and remove the trailing argument comma in propertyHelper.js.

This makes the harness ES5-friendlier and allows nearly a third of the
tests to pass on old ES5 engines from a baseline of ~8.7% of all
negative cases - assert.js is a key blocker.
@ivankra ivankra requested a review from a team as a code owner March 25, 2026 14:37
@linusg
Copy link
Member

linusg commented Mar 25, 2026

Ref #3032 #3196

@ivankra
Copy link
Author

ivankra commented Mar 25, 2026

I'm proposing only to change these two high-impact places. There's a lot of more of ES6 in the harness but the marginal effect of changing anything else is pretty small.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

love the direction :-)

Copy link
Contributor

@Ms2ger Ms2ger left a comment

Choose a reason for hiding this comment

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

I don't love the direction, but I think the changes here are acceptable.

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.

4 participants