Skip to content

Commit 6d46a8b

Browse files
mhegazymhegazy
committed
more code review comments
1 parent 99305e7 commit 6d46a8b

File tree

2 files changed

+67
-103
lines changed

2 files changed

+67
-103
lines changed

‎src/emitter.ts

Lines changed: 37 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ const extendConflictsBaseTypes: Record<string, { extendType: string[], memberNam
2828
"AudioContext": { extendType: ["OfflineContext"], memberNames: new Set(["suspend"]) },
2929
"HTMLCollection": { extendType: ["HTMLFormControlsCollection"], memberNames: new Set(["namedItem"]) },
3030
};
31-
const evetTypeMap: Record<string, string> = {
31+
const eventTypeMap: Record<string, string> = {
3232
"abort": "UIEvent",
3333
"complete": "Event",
3434
"click": "MouseEvent",
@@ -60,6 +60,7 @@ function createTextWriter(newLine: string) {
6060
let output: string;
6161
let indent: number;
6262
let lineStart: boolean;
63+
/** print declarations conflicting with base interface to a side list to write them under a diffrent name later */
6364
let stack: { content: string, indent: number }[] = [];
6465

6566
const indentStrings: string[] = ["", " "];
@@ -95,7 +96,6 @@ function createTextWriter(newLine: string) {
9596
reset();
9697

9798
return {
98-
9999
reset: reset,
100100

101101
resetIndent() { indent = 0; },
@@ -105,8 +105,6 @@ function createTextWriter(newLine: string) {
105105
print: write,
106106
printLine(c: string) { writeLine(); write(c); },
107107

108-
printWithAddedIndent(c: string) { this.increaseIndent(); this.printLine(c); this.decreaseIndent(); },
109-
110108
clearStack() { stack = []; },
111109
stackIsEmpty() { return stack.length === 0; },
112110
printLineToStack(content: string) { stack.push({ content, indent }); },
@@ -145,7 +143,7 @@ export function emitWebIDl(webidl: Browser.WebIdl, flavor: Flavor) {
145143
const allTypeDefsMap = new Set(webidl.typedefs && webidl.typedefs.typedef.map(td => td["new-type"]));
146144

147145
/// Event name to event type map
148-
const eNameToEType = arrayToMap(flatMap(allNonCallbackInterfaces, i => i.events ? i.events.event : []), e => e.name, e => evetTypeMap[e.name] || e.type);
146+
const eNameToEType = arrayToMap(flatMap(allNonCallbackInterfaces, i => i.events ? i.events.event : []), e => e.name, e => eventTypeMap[e.name] || e.type);
149147

150148
/// Tag name to element name map
151149
const tagNameToEleName = getTagNameToElementNameMap();
@@ -512,7 +510,9 @@ export function emitWebIDl(webidl: Browser.WebIdl, flavor: Flavor) {
512510
function emitCallBackInterface(i: Browser.Interface) {
513511
if (i.name === "EventListener") {
514512
printer.printLine(`interface ${i.name} {`);
515-
printer.printWithAddedIndent("(evt: Event): void;");
513+
printer.increaseIndent();
514+
printer.printLine("(evt: Event): void;");
515+
printer.decreaseIndent();
516516
printer.printLine("}");
517517
}
518518
else {
@@ -612,27 +612,23 @@ export function emitWebIDl(webidl: Browser.WebIdl, flavor: Flavor) {
612612
}
613613
}
614614

615-
function emitComments(entity: { comment?: string; deprecated?: 1 } | undefined, print: (s: string) => void) {
616-
if (entity) {
617-
if (entity.comment) {
618-
print(entity.comment);
619-
}
620-
if (entity.deprecated) {
621-
print(`/** @deprecated */`);
622-
}
615+
function emitComments(entity: { comment?: string; deprecated?: 1 }, print: (s: string) => void) {
616+
if (entity.comment) {
617+
print(entity.comment);
618+
}
619+
if (entity.deprecated) {
620+
print(`/** @deprecated */`);
623621
}
624622
}
625623

626624
function emitProperties(prefix: string, emitScope: EmitScope, i: Browser.Interface, conflictedMembers: Set<string>) {
627625
// Note: the schema file shows the property doesn't have "static" attribute,
628626
// therefore all properties are emited for the instance type.
629-
if (emitScope !== EmitScope.StaticOnly) {
630-
if (i.properties) {
631-
mapToArray(i.properties.property)
632-
.filter(p => !isCovariantEventHandler(i, p))
633-
.sort(compareName)
634-
.forEach(p => emitProperty(prefix, i, emitScope, p, conflictedMembers));
635-
}
627+
if (emitScope !== EmitScope.StaticOnly && i.properties) {
628+
mapToArray(i.properties.property)
629+
.filter(p => !isCovariantEventHandler(i, p))
630+
.sort(compareName)
631+
.forEach(p => emitProperty(prefix, i, emitScope, p, conflictedMembers));
636632
}
637633
}
638634

@@ -680,14 +676,9 @@ export function emitWebIDl(webidl: Browser.WebIdl, flavor: Flavor) {
680676
function emitMethods(prefix: string, emitScope: EmitScope, i: Browser.Interface, conflictedMembers: Set<string>) {
681677
// If prefix is not empty, then this is the global declare function addEventListener, we want to override this
682678
// Otherwise, this is EventTarget.addEventListener, we want to keep that.
683-
function mFilter(m: Browser.Method) {
684-
return matchScope(emitScope, m) &&
685-
!(prefix !== "" && (m.name === "addEventListener" || m.name === "removeEventListener"));
686-
}
687-
688679
if (i.methods) {
689680
mapToArray(i.methods.method)
690-
.filter(mFilter)
681+
.filter(m => matchScope(emitScope, m) && !(prefix !== "" && (m.name === "addEventListener" || m.name === "removeEventListener")))
691682
.sort(compareName)
692683
.forEach(m => emitMethod(prefix, i, m, conflictedMembers));
693684
}
@@ -710,8 +701,7 @@ export function emitWebIDl(webidl: Browser.WebIdl, flavor: Flavor) {
710701
/// Emit all members of every interfaces at the root level.
711702
/// Called only once on the global polluter object
712703
function emitAllMembers(i: Browser.Interface) {
713-
const prefix = "declare var ";
714-
emitMembers(prefix, EmitScope.All, i);
704+
emitMembers(/*prefix*/ "declare var ", EmitScope.All, i);
715705

716706
for (const relatedIName of iNameToIDependList[i.name]) {
717707
const i = allInterfacesMap[relatedIName];
@@ -722,52 +712,43 @@ export function emitWebIDl(webidl: Browser.WebIdl, flavor: Flavor) {
722712
}
723713

724714
function emitEventHandlers(prefix: string, i: Browser.Interface) {
725-
function getOptionsType(addOrRemove: string) {
726-
return addOrRemove === "add" ? "AddEventListenerOptions" : "EventListenerOptions";
727-
}
728-
729715
const fPrefix = prefix.startsWith("declare var") ? "declare function " : "";
730716

731-
function emitTypedEventHandler(prefix: string, addOrRemove: string, iParent: Browser.Interface) {
732-
printer.printLine(`${prefix}${addOrRemove}EventListener<K extends keyof ${iParent.name}EventMap>(type: K, listener: (this: ${i.name}, ev: ${iParent.name}EventMap[K]) => any, options?: boolean | ${getOptionsType(addOrRemove)}): void;`);
717+
for (const addOrRemove of ["add", "remove"]) {
718+
const optionsType = addOrRemove === "add" ? "AddEventListenerOptions" : "EventListenerOptions";
719+
if (tryEmitTypedEventHandlerForInterface(addOrRemove, optionsType)) {
720+
// only emit the string event handler if we just emited a typed handler
721+
printer.printLine(`${fPrefix}${addOrRemove}EventListener(type: string, listener: EventListenerOrEventListenerObject, options?: boolean | ${optionsType}): void;`);
722+
}
733723
}
734724

735-
function emitStringEventHandler(addOrRemove: string) {
736-
printer.printLine(`${fPrefix}${addOrRemove}EventListener(type: string, listener: EventListenerOrEventListenerObject, options?: boolean | ${getOptionsType(addOrRemove)}): void;`);
725+
return;
726+
727+
function emitTypedEventHandler(prefix: string, addOrRemove: string, iParent: Browser.Interface, optionsType: string) {
728+
printer.printLine(`${prefix}${addOrRemove}EventListener<K extends keyof ${iParent.name}EventMap>(type: K, listener: (this: ${i.name}, ev: ${iParent.name}EventMap[K]) => any, options?: boolean | ${optionsType}): void;`);
737729
}
738730

739-
function tryEmitTypedEventHandlerForInterface(addOrRemove: string) {
731+
function tryEmitTypedEventHandlerForInterface(addOrRemove: string, optionsType: string) {
740732
if (iNameToEhList[i.name] && iNameToEhList[i.name].length) {
741-
emitTypedEventHandler(fPrefix, addOrRemove, i);
733+
emitTypedEventHandler(fPrefix, addOrRemove, i, optionsType);
742734
return true;
743735
}
744736
if (iNameToEhParents[i.name] && iNameToEhParents[i.name].length) {
745737
iNameToEhParents[i.name]
746738
.sort(compareName)
747-
.forEach(i => emitTypedEventHandler(fPrefix, addOrRemove, i));
739+
.forEach(i => emitTypedEventHandler(fPrefix, addOrRemove, i, optionsType));
748740
return true;
749741
}
750742
return false;
751743
}
752-
753-
function emitEventHandler(addOrRemove: string) {
754-
if (tryEmitTypedEventHandlerForInterface(addOrRemove)) {
755-
// only emit the string event handler if we just emited a typed handler
756-
emitStringEventHandler(addOrRemove);
757-
}
758-
}
759-
760-
emitEventHandler("add");
761-
emitEventHandler("remove");
762744
}
763745

764746
function emitConstructorSignature(i: Browser.Interface) {
765747
const constructor = typeof i.constructor === "object" ? i.constructor : undefined;
766748

767-
emitComments(constructor, s => printer.print(s));
768-
769749
// Emit constructor signature
770750
if (constructor) {
751+
emitComments(constructor, s => printer.print(s));
771752
emitSignatures(constructor, "", "new", s => printer.printLine(s));
772753
}
773754
else {
@@ -782,7 +763,7 @@ export function emitWebIDl(webidl: Browser.WebIdl, flavor: Flavor) {
782763
printer.printLine(`prototype: ${i.name};`);
783764
emitConstructorSignature(i);
784765
emitConstants(i);
785-
emitMembers("", EmitScope.StaticOnly, i);
766+
emitMembers(/*prefix*/ "", EmitScope.StaticOnly, i);
786767

787768
printer.decreaseIndent();
788769
printer.printLine("};");
@@ -793,7 +774,9 @@ export function emitWebIDl(webidl: Browser.WebIdl, flavor: Flavor) {
793774
const nc = i["named-constructor"];
794775
if (nc) {
795776
printer.printLine(`declare var ${nc.name}: {`);
796-
nc.signature.forEach(s => printer.printWithAddedIndent(`new(${s.param ? paramsToString(s.param) : ""}): ${i.name};`));
777+
printer.increaseIndent();
778+
nc.signature.forEach(s => printer.printLine(`new(${s.param ? paramsToString(s.param) : ""}): ${i.name};`));
779+
printer.decreaseIndent();
797780
printer.printLine(`};`);
798781
}
799782
}

‎src/index.ts

Lines changed: 30 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -5,30 +5,7 @@ import { filter, merge, filterProperties } from "./helpers";
55
import { Flavor, emitWebIDl } from "./emitter";
66

77
function emitDomWorker(webidl: Browser.WebIdl, knownWorkerTypes: Set<string>, tsWorkerOutput: string) {
8-
const worker: Browser.WebIdl = {
9-
"callback-functions": {
10-
"callback-function": {}
11-
},
12-
"callback-interfaces": {
13-
"interface": {}
14-
},
15-
"dictionaries": {
16-
"dictionary": {}
17-
},
18-
"enums": {
19-
"enum": {}
20-
},
21-
"interfaces": {
22-
"interface": {}
23-
},
24-
"mixins": {
25-
"mixin": {}
26-
},
27-
"typedefs": {
28-
"typedef": []
29-
}
30-
};
31-
8+
const worker = getEmptyWebIDL();
329
const isKnownWorkerName = (o: { name: string }) => knownWorkerTypes.has(o.name);
3310

3411
if (webidl["callback-functions"]) worker["callback-functions"]!["callback-function"] = filterProperties(webidl["callback-functions"]!["callback-function"], isKnownWorkerName);
@@ -59,6 +36,32 @@ function emitES6DomIterators(webidl: Browser.WebIdl, tsWebES6Output: string) {
5936
fs.writeFileSync(tsWebES6Output, emitWebIDl(webidl, Flavor.ES6Iterators));
6037
}
6138

39+
function getEmptyWebIDL(): Browser.WebIdl {
40+
return {
41+
"callback-functions": {
42+
"callback-function": {}
43+
},
44+
"callback-interfaces": {
45+
"interface": {}
46+
},
47+
"dictionaries": {
48+
"dictionary": {}
49+
},
50+
"enums": {
51+
"enum": {}
52+
},
53+
"interfaces": {
54+
"interface": {}
55+
},
56+
"mixins": {
57+
"mixin": {}
58+
},
59+
"typedefs": {
60+
"typedef": []
61+
}
62+
}
63+
}
64+
6265
function emitDom() {
6366
const __SOURCE_DIRECTORY__ = __dirname;
6467
const inputFolder = path.join(__SOURCE_DIRECTORY__, "../", "inputfiles");
@@ -94,33 +97,11 @@ function emitDom() {
9497
emitES6DomIterators(webidl, tsWebES6Output);
9598

9699
function prune(obj: Browser.WebIdl, template: Partial<Browser.WebIdl>): Browser.WebIdl {
97-
const result: Browser.WebIdl = {
98-
"callback-functions": {
99-
"callback-function": {}
100-
},
101-
"callback-interfaces": {
102-
"interface": {}
103-
},
104-
"dictionaries": {
105-
"dictionary": {}
106-
},
107-
"enums": {
108-
"enum": {}
109-
},
110-
"interfaces": {
111-
"interface": {}
112-
},
113-
"mixins": {
114-
"mixin": {}
115-
},
116-
"typedefs": {
117-
"typedef": []
118-
}
119-
};
100+
const result = getEmptyWebIDL();
120101

121102
if (obj["callback-functions"]) result["callback-functions"]!["callback-function"] = filterProperties(obj["callback-functions"]!["callback-function"], (cb) => !(template["callback-functions"] && template["callback-functions"]!["callback-function"][cb.name]));
122103
if (obj["callback-interfaces"]) result["callback-interfaces"]!.interface = filterInterface(obj["callback-interfaces"]!.interface, template["callback-interfaces"] && template["callback-interfaces"]!.interface);
123-
if (obj.dictionaries) result.dictionaries!.dictionary = filterDictinary(obj.dictionaries.dictionary, template.dictionaries && template.dictionaries.dictionary);
104+
if (obj.dictionaries) result.dictionaries!.dictionary = filterDictionary(obj.dictionaries.dictionary, template.dictionaries && template.dictionaries.dictionary);
124105
if (obj.enums) result.enums!.enum = filterEnum(obj.enums.enum, template.enums && template.enums.enum);
125106
if (obj.mixins) result.mixins!.mixin = filterInterface(obj.mixins.mixin, template.mixins && template.mixins.mixin);
126107
if (obj.interfaces) result.interfaces!.interface = filterInterface(obj.interfaces.interface, template.interfaces && template.interfaces.interface);
@@ -142,7 +123,7 @@ function emitDom() {
142123
return result;
143124
}
144125

145-
function filterDictinary(dictinaries: Record<string, Browser.Dictionary>, template: Record<string, Browser.Dictionary> | undefined) {
126+
function filterDictionary(dictinaries: Record<string, Browser.Dictionary>, template: Record<string, Browser.Dictionary> | undefined) {
146127
if (!template) return dictinaries;
147128
const result = filterProperties(dictinaries, i => !template[i.name]);
148129
for (const k in result) {

0 commit comments

Comments
 (0)