Skip to content

Add missing extern composition methods for global Set object #4213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add missing extern composition methods for global Set object #4213

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 74 additions & 7 deletions externs/es6_collections.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ WeakMap.prototype.set = function(key, value) {};
* @implements {Iterable<VALUE>}
* @template VALUE
* @nosideeffects
* @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set
* @see https://tc39.es/ecma262/multipage/keyed-collections.html#sec-set-iterable
*/
function Set(opt_iterable) {}

Expand All @@ -227,24 +227,31 @@ function Set(opt_iterable) {}
* @return {THIS}
* @this {THIS}
* @template THIS
* @modifies {this}
* @see https://tc39.es/ecma262/multipage/keyed-collections.html#sec-set.prototype.add
*/
Set.prototype.add = function(value) {};

/**
* @return {void}
* @modifies {this}
* @see https://tc39.es/ecma262/multipage/keyed-collections.html#sec-set.prototype.clear
*/
Set.prototype.clear = function() {};

/**
* @param {VALUE} value
* @return {boolean}
* @modifies {this}
* @see https://tc39.es/ecma262/multipage/keyed-collections.html#sec-set.prototype.delete
*/
Set.prototype.delete = function(value) {};

/**
* @return {!IteratorIterable<!Array<VALUE>>} Where each array has two entries:
* [value, value]
* @nosideeffects
* @see https://tc39.es/ecma262/multipage/keyed-collections.html#sec-set.prototype.values
*/
Set.prototype.entries = function() {};

Expand All @@ -253,39 +260,99 @@ Set.prototype.entries = function() {};
* @param {THIS=} opt_thisArg
* @this {SET}
* @template SET,THIS
* @see https://tc39.es/ecma262/multipage/keyed-collections.html#sec-set.prototype.foreach
*/
Set.prototype.forEach = function(callback, opt_thisArg) {};

/**
* @param {VALUE} value
* @return {boolean}
* @nosideeffects
* @see https://tc39.es/ecma262/multipage/keyed-collections.html#sec-set.prototype.has
*/
Set.prototype.has = function(value) {};

/**
* @type {number} (readonly)
*/
Set.prototype.size;

/**
* @return {!IteratorIterable<VALUE>}
* @nosideeffects
* @see https://tc39.es/ecma262/multipage/keyed-collections.html#sec-set.prototype.keys
*/
Set.prototype.keys = function() {};

/**
* @return {!IteratorIterable<VALUE>}
* @nosideeffects
* @see https://tc39.es/ecma262/multipage/keyed-collections.html#sec-set.prototype.values
*/
Set.prototype.values = function() {};

/**
* @param {!Set} other
* @return {!Set}
* @nosideeffects
* @see https://tc39.es/proposal-set-methods/#sec-set.prototype.union
*/
Set.prototype.union = function(other) {};

/**
* @param {!Set} other
* @return {!Set}
* @nosideeffects
* @see https://tc39.es/proposal-set-methods/#sec-set.prototype.intersection
*/
Set.prototype.intersection = function(other) {};

/**
* @param {!Set} other
* @return {!Set}
* @nosideeffects
* @see https://tc39.es/proposal-set-methods/#sec-set.prototype.difference
*/
Set.prototype.difference = function(other) {};

/**
* @param {!Set} other
* @return {!Set}
* @nosideeffects
* @see https://tc39.es/proposal-set-methods/#sec-set.prototype.symmetricdifference
*/
Set.prototype.symmetricDifference = function(other) {};

/**
* @param {!Set} other
* @return {boolean}
* @nosideeffects
* @see https://tc39.es/proposal-set-methods/#sec-set.prototype.issupersetof
*/
Set.prototype.isSupersetOf = function(other) {};

/**
* @param {!Set} other
* @return {boolean}
* @nosideeffects
* @see https://tc39.es/proposal-set-methods/#sec-set.prototype.issubsetof
*/
Set.prototype.isSubsetOf = function(other) {};

/**
* @param {!Set} other
* @return {boolean}
* @nosideeffects
* @see https://tc39.es/proposal-set-methods/#sec-set.prototype.isdisjointfrom
*/
Set.prototype.isDisjointFrom = function(other) {};

/**
* @return {!Iterator<VALUE>}
* @see https://tc39.es/ecma262/multipage/keyed-collections.html#sec-set.prototype-%symbol.iterator%
*/
Set.prototype[Symbol.iterator] = function() {};


/**
* @type {number} (readonly)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not @const {number} if it's readonly?

Copy link
Contributor Author

@ecrider ecrider Feb 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I must say it's a very stable codebase and one that can still compete to this day with new fancy bleeding-edge compilers, bundlers, transpilers - even in terms of raw performance from optimized code, which is more than impressive, it's insane. A little out of fashion maybe, for most seem overly complicated, rough around the edges, demanding to adapt, pale in comparison I heard. There are thousands of articles on how to make friends with TypeScript for example, but for Closure-Compiler there is almost nothing there - there is no mastering it without diving deep into the source code. That said, as a development utility it's nearly impossible to replace because of how reliable it is, a striking contrast to the majority of tools that are used in the JavaScript supply ecosystem.

I like to think it saves me $10k for every single thing it doesn't break when deployed into "enterprise-grade production pipelines" - and just that would make me a billionaire because there are a lot of things that can break at any time in those piles of corporate manure. Problems can be solved by simply introducing some new dependencies, worked well so everyone kept doing it - if something works, it works... until it doesn't.

To be honest most of those infrastructures keeping our lights on is already broken beyond repair at a fundamental level, waiting to implode - looks pretty solid from up far, but when you get curious, go down there, look close enough - you see a graveyard, a tomb filled with nothing but corpses conducting electrical current, empty husks on a life support from daemonized routines and terabytes of cache in random-access memory. All those places maintained by skeleton crews of underpaid people - gnomes, good spirits coming out at night and conducting minor repairs, tending to those failing server racks, basically carrying our glorious, highly advanced civilisation on their backs, postponing doomsday and saving the world by sitting there in the dark and knowing exactly what machines can and what machines cannot be rebooted ever. Do not despair though, calvary is coming - they will soon be liberated, freed from their dungeons by none other than Sam Altman himself arriving on a white horse, with a pardon letter, universal basic income and a ChatGPT terminal strapped to his saddlebag. Glory to mankind.

This is not a rant, it's a love letter to Closure Compiler, I needed that, just let me be.
But I digress...

Why not @const {number} if it's readonly?

There are some ES6 polyfills being dynamically injected by the compiler, early enough to be included in general code optimisation, they also rely on extern definitions. Basically "size" property is supposed to be read-only, but it can't be really because Set polyfills need to be able to reassign this value under the hood.

closure-compiler/src/com/google/javascript/jscomp/js/es6/set.js

Lines 97 to 100 in 0b52016

// Note: this property should not be changed. If we're willing to give up
// ES3 support, we could define it as a property directly. It should be
// marked readonly if such an annotation ever comes into existence.
this.size = this.map_.size;

My understanding is that we should mark it as read-only property, but without declaring it as actual constant - doing so would trigger warnings in advanced mode that would be hard to debug for users since it's internal matter - they wouldn't be able to find the offender in their codebase, nor suppress it.

When defining as constant and trying to build the entire thing there are warnings in place complaining about it.

src/com/google/javascript/jscomp/js/es6/set.js:100:9: WARNING - [JSC_CONSTANT_PROPERTY_REASSIGNED_VALUE] constant property size assigned a value more than once
Initialized at externs.zip//es6_collections.js:355:14
  100|     this.size = this.map_.size;
                ^^^^

src/com/google/javascript/jscomp/js/es6/set.js:109:9: WARNING - [JSC_CONSTANT_PROPERTY_REASSIGNED_VALUE] constant property size assigned a value more than once
Initialized at src/com/google/javascript/jscomp/js/es6/set.js:100:9
  109|     this.size = this.map_.size;
                ^^^^

src/com/google/javascript/jscomp/js/es6/set.js:117:9: WARNING - [JSC_CONSTANT_PROPERTY_REASSIGNED_VALUE] constant property size assigned a value more than once
Initialized at src/com/google/javascript/jscomp/js/es6/set.js:100:9
  117|     this.size = this.map_.size;
                ^^^^

src/com/google/javascript/jscomp/js/es6/set.js:125:9: WARNING - [JSC_CONSTANT_PROPERTY_REASSIGNED_VALUE] constant property size assigned a value more than once
Initialized at src/com/google/javascript/jscomp/js/es6/set.js:100:9
  125|     this.size = 0;
                ^^^^

* @see https://tc39.es/ecma262/multipage/keyed-collections.html#sec-get-set.prototype.size
*/
Set.prototype.size;

/**
* @constructor @struct
Expand Down