Closed Bug 1222552 Opened 5 years ago Closed 1 year ago

Remove String generics

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox45 --- wontfix
firefox68 --- fixed

People

(Reporter: arai, Assigned: evilpie)

References

(Blocks 1 open bug)

Details

(Keywords: addon-compat, dev-doc-complete, site-compat)

Attachments

(2 files)

String generics are non-standard SpiderMonkey extension and they should be removed.
  * String.charAt
  * String.charCodeAt
  * String.concat
  * String.contains
  * String.endsWith
  * String.includes
  * String.indexOf
  * String.lastIndexOf
  * String.localeCompare
  * String.match
  * String.normalize
  * String.replace
  * String.search
  * String.slice
  * String.split
  * String.startsWith
  * String.substr
  * String.substring
  * String.toLocaleLowerCase
  * String.toLocaleUpperCase
  * String.toLowerCase
  * String.toUpperCase
  * String.trim
  * String.trimLeft
  * String.trimRight
See Also: → 1222547
Depends on: 1319926
Depends on: 1339777
I think it's time we seriously investigate this and bug 1222547. The String variant is probably less used, but we definitely need to add some telemetry. The static String functions already have warnings, but the Array methods don't. This also means those are still used in Firefox code.

The telemetry for this produced quite interesting results.

method usage
toLowerCase 519106 (97.75%)
concat 4898 (0.92%)
slice 3490 (0.66%)
trim 3336 (0.63%)

The rest is basically at 0%. source

I wonder if we should just go ahead and remove all of the rest? Except for toLowerCase even the rest feels quite low.

There's an old Mootools version in-tree, which, as part of its initialisation, seems to call String.toLowerCase. Maybe this Mootools version is still used on the Web and that's why toLowerCase is so prominent in the telemetry data? In that case it's safe to remove toLowerCase, because Mootools will add it back.

(In reply to André Bargull [:anba] from comment #4)

There's an old Mootools version in-tree, which, as part of its initialisation, seems to call String.toLowerCase. Maybe this Mootools version is still used on the Web and that's why toLowerCase is so prominent in the telemetry data? In that case it's safe to remove toLowerCase, because Mootools will add it back.

Wow good find! I really hope this is true and we can just remove everything.

That really is an amazing find, anba. Wow.

Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Assignee: nobody → evilpies

== Change summary for alert #20084 (as of Sat, 23 Mar 2019 05:37:40 GMT) ==

Improvements:

1% Base Content JS linux64 pgo stylo 3,999,578.67 -> 3,968,909.33
1% Base Content JS linux64-pgo-qr opt stylo 3,999,470.67 -> 3,968,886.67
1% Base Content JS windows10-64-pgo-qr opt stylo 4,073,260.00 -> 4,046,252.00
1% Base Content JS windows10-64 pgo stylo 4,073,281.33 -> 4,046,340.00
1% Base Content JS osx-10-10 opt stylo 4,000,505.33 -> 3,974,025.33
0% Base Content JS windows7-32 pgo stylo 3,165,454.67 -> 3,150,896.00

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=20084

Sorry this slipped through somehow in time for Firefox 68.

Announced on Firefox 68 release notes: https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/68#JavaScript

Updated the warning page that is shown in earlier Firefox versions: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Deprecated_String_generics

Removed the entire "String generics" section from the String API docs: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String

You need to log in before you can comment on or make changes to this bug.