fix: guard against duplicate TouchBarItem IDs (electron#22272)

erickzhao · web-flow · commit 85ef762269a1 · 2020-02-26T22:12:56.000Z
* feat: Add OtherItemsProxy touchbar item

* review!

* fix: guard against duplicate TouchBarItem IDs

* add spec
diff --git a/lib/browser/api/touch-bar.js b/lib/browser/api/touch-bar.js
@@ -53,6 +53,7 @@ class TouchBar extends EventEmitter {
     }
 
     let hasOtherItemsProxy = false
+    const idSet = new Set()
     items.forEach((item) => {
       if (!(item instanceof TouchBarItem)) {
         throw new Error('Each item must be an instance of TouchBarItem')
@@ -65,6 +66,12 @@ class TouchBar extends EventEmitter {
           throw new Error('Must only have one OtherItemsProxy per TouchBar')
         }
       }
+
+      if (!idSet.has(item.id)) {
+        idSet.add(item.id)
+      } else {
+        throw new Error('Cannot add a single instance of TouchBarItem multiple times in a TouchBar')
+      }
     })
 
     // register in separate loop after all items are validated
diff --git a/spec-main/api-touch-bar-spec.ts b/spec-main/api-touch-bar-spec.ts
@@ -39,6 +39,14 @@ describe('TouchBar module', () => {
     }).to.throw('Must only have one OtherItemsProxy per TouchBar')
   })
 
+  it('throws an error if the same TouchBarItem is added multiple times', () => {
+    expect(() => {
+      const item = new TouchBarLabel({ label: 'Label' })
+      const touchBar = new TouchBar({ items: [item, item] })
+      touchBar.toString()
+    }).to.throw('Cannot add a single instance of TouchBarItem multiple times in a TouchBar')
+  })
+
   describe('BrowserWindow behavior', () => {
     let window: BrowserWindow
 

Original file line number	Diff line number	Diff line change
`@@ -53,6 +53,7 @@ class TouchBar extends EventEmitter {`
`53`	`53`	`}`
`54`	`54`
`55`	`55`	`let hasOtherItemsProxy = false`
	`56`	`+ const idSet = new Set()`
`56`	`57`	`items.forEach((item) => {`
`57`	`58`	`if (!(item instanceof TouchBarItem)) {`
`58`	`59`	`throw new Error('Each item must be an instance of TouchBarItem')`
`@@ -65,6 +66,12 @@ class TouchBar extends EventEmitter {`
`65`	`66`	`throw new Error('Must only have one OtherItemsProxy per TouchBar')`
`66`	`67`	`}`
`67`	`68`	`}`
	`69`	`+`
	`70`	`+ if (!idSet.has(item.id)) {`
	`71`	`+ idSet.add(item.id)`
	`72`	`+ } else {`
	`73`	`+ throw new Error('Cannot add a single instance of TouchBarItem multiple times in a TouchBar')`
	`74`	`+ }`
`68`	`75`	`})`
`69`	`76`
`70`	`77`	`// register in separate loop after all items are validated`