Skip to content

Commit d4aea91

Browse files
lauraharkeravelad
authored andcommitted
refactor: fix upcoming Closure Compiler type errors on Symbol.iterator (#8851)
Closure Compiler will soon start typechecking well-known symbol properties, such as Symbol.iterator - see google/closure-compiler#1737. This will cause some type errors in existing code that implements `Iterable` (for context, I ran into these errors in google's internal repo) that is missing a Symbol.iterator override or `@override` annotation
1 parent 20feb0f commit d4aea91

File tree

5 files changed

+35
-9
lines changed

5 files changed

+35
-9
lines changed

build/generateExterns.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,15 @@ function createExternMethod(node) {
490490
if (node.static) {
491491
methodString += 'static ';
492492
}
493-
methodString += id + '(' + params.join(', ') + ') {}';
493+
if (node.key.type === 'Identifier') {
494+
methodString += id;
495+
} else {
496+
assert.equal(
497+
node.key.type, 'MemberExpression',
498+
'Unexpected exported member name in exported class!');
499+
methodString += `[${id}]`;
500+
}
501+
methodString += '(' + params.join(', ') + ') {}';
494502
return methodString;
495503
}
496504

@@ -566,6 +574,11 @@ function createExternAssignment(name, node, alwaysIncludeConstructor) {
566574
// constructor in some situations.
567575
if (member.key.name == 'constructor' && alwaysIncludeConstructor) {
568576
// Fall through and generate externs.
577+
} else if (member.key.type === 'MemberExpression' &&
578+
getIdentifierString(member.key) === 'Symbol.iterator') {
579+
// Symbol.iterator is needed to implement the Iterable interface for
580+
// Closure Compiler, so always assume it is exported.
581+
// Closure Compiler does not allow putting an explicit @export.
569582
} else {
570583
// Skip extern generation.
571584
continue;

externs/domstringlist.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,9 @@
2626
* @suppress {duplicate}
2727
*/
2828
var DOMStringList = function() {};
29+
30+
/**
31+
* @override
32+
* @return {!Iterator<string>}
33+
*/
34+
DOMStringList.prototype[Symbol.iterator] = function() {};

externs/texttrackcuelist.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,6 @@
2424
* @suppress {duplicate}
2525
*/
2626
var TextTrackCueList = function() {};
27+
28+
/** @override */
29+
TextTrackCueList.prototype[Symbol.iterator] = function() {};

lib/dash/segment_template.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -718,7 +718,7 @@ shaka.dash.SegmentTemplate = class {
718718
*
719719
* @extends shaka.media.SegmentIndex
720720
* @implements {shaka.util.IReleasable}
721-
* @implements {Iterable<!shaka.media.SegmentReference>}
721+
* @implements {Iterable<?shaka.media.SegmentReference>}
722722
*
723723
* @private
724724
*

lib/media/segment_index.js

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ goog.require('shaka.util.Timer');
2020
*
2121
* @implements {shaka.extern.SegmentIndex}
2222
* @implements {shaka.util.IReleasable}
23-
* @implements {Iterable<!shaka.media.SegmentReference>}
23+
* @implements {Iterable<?shaka.media.SegmentReference>}
2424
* @export
2525
*/
2626
shaka.media.SegmentIndex = class {
@@ -414,7 +414,10 @@ shaka.media.SegmentIndex = class {
414414
}
415415

416416

417-
/** @return {!shaka.media.SegmentIterator} */
417+
/**
418+
* @return {!shaka.media.SegmentIterator}
419+
* @override
420+
*/
418421
[Symbol.iterator]() {
419422
const iter = this.getIteratorForTime(0);
420423
goog.asserts.assert(iter != null, 'Iterator for 0 should never be null!');
@@ -555,18 +558,18 @@ if (goog.DEBUG) {
555558
/**
556559
* An iterator over a SegmentIndex's references.
557560
*
558-
* @implements {Iterator<shaka.media.SegmentReference>}
561+
* @implements {Iterator<?shaka.media.SegmentReference>}
559562
* @export
560563
*/
561564
shaka.media.SegmentIterator = class {
562565
/**
563-
* @param {shaka.media.SegmentIndex} segmentIndex
566+
* @param {!shaka.media.SegmentIndex} segmentIndex
564567
* @param {number} index
565568
* @param {number} partialSegmentIndex
566569
* @param {boolean} reverse
567570
*/
568571
constructor(segmentIndex, index, partialSegmentIndex, reverse) {
569-
/** @private {shaka.media.SegmentIndex} */
572+
/** @private {!shaka.media.SegmentIndex} */
570573
this.segmentIndex_ = segmentIndex;
571574

572575
/** @private {number} */
@@ -596,7 +599,7 @@ shaka.media.SegmentIterator = class {
596599
}
597600

598601
/**
599-
* @return {shaka.media.SegmentReference}
602+
* @return {?shaka.media.SegmentReference}
600603
* @export
601604
*/
602605
current() {
@@ -625,6 +628,7 @@ shaka.media.SegmentIterator = class {
625628
/**
626629
* @override
627630
* @export
631+
* @return {!IIterableResult<?shaka.media.SegmentReference>}
628632
*/
629633
next() {
630634
const ref = this.segmentIndex_.get(this.currentPosition_);
@@ -708,7 +712,7 @@ shaka.media.SegmentIterator = class {
708712
*
709713
* @extends shaka.media.SegmentIndex
710714
* @implements {shaka.util.IReleasable}
711-
* @implements {Iterable<!shaka.media.SegmentReference>}
715+
* @implements {Iterable<?shaka.media.SegmentReference>}
712716
* @export
713717
*/
714718
shaka.media.MetaSegmentIndex = class extends shaka.media.SegmentIndex {

0 commit comments

Comments
 (0)