Disable auto-fix .onmessage in prefer-add-event-listener rule (#543)

sindresorhus · Feb 13, 2020 · 96af562 · 96af562
1 parent a5e5405
commit 96af562
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 32 deletions.
diff --git a/rules/prefer-add-event-listener.js b/rules/prefer-add-event-listener.js
@@ -3,14 +3,17 @@ const getDocumentationUrl = require('./utils/get-documentation-url');
 const domEventsJson = require('./utils/dom-events.json');
 
 const message = 'Prefer `{{replacement}}` over `{{method}}`.{{extra}}';
-const beforeUnloadMessage = ' Use `event.preventDefault(); event.returnValue = \'foo\'` to trigger the prompt.';
+const extraMessages = {
+	beforeunload: 'Use `event.preventDefault(); event.returnValue = \'foo\'` to trigger the prompt.',
+	message: 'Note that there is difference between `SharedWorker#onmessage` and `SharedWorker#addEventListener(\'message\')`.'
+};
 
-const nestedEvents = Object.keys(domEventsJson).map(key => domEventsJson[key]);
+const nestedEvents = Object.values(domEventsJson);
 const eventTypes = new Set(nestedEvents.reduce((accumulatorEvents, events) => accumulatorEvents.concat(events), []));
 const getEventMethodName = memberExpression => memberExpression.property.name;
 const getEventTypeName = eventMethodName => eventMethodName.slice('on'.length);
 
-const fix = (fixer, sourceCode, assignmentNode, memberExpression) => {
+const fixCode = (fixer, sourceCode, assignmentNode, memberExpression) => {
 	const eventTypeName = getEventTypeName(getEventMethodName(memberExpression));
 	const eventObjectCode = sourceCode.getText(memberExpression.object);
 	const fncCode = sourceCode.getText(assignmentNode.right);
@@ -106,28 +109,34 @@ const create = context => {
 				return;
 			}
 
-			const problem = {
-				node,
-				message,
-				data: {
-					replacement: 'addEventListener',
-					method: eventMethodName,
-					extra: ''
-				}
-			};
+			let replacement = 'addEventListener';
+			let extra = '';
+			let fix;
 
 			if (isClearing(assignedExpression)) {
-				problem.data.replacement = 'removeEventListener';
+				replacement = 'removeEventListener';
 			} else if (
 				eventTypeName === 'beforeunload' &&
 				!shouldFixBeforeUnload(assignedExpression, nodeReturnsSomething)
 			) {
-				problem.data.extra = beforeUnloadMessage;
+				extra = extraMessages.beforeunload;
+			} else if (eventTypeName === 'message') {
+				// Disable `onmessage` fix, see #537
+				extra = extraMessages.message;
 			} else {
-				problem.fix = fixer => fix(fixer, context.getSourceCode(), node, memberExpression);
+				fix = fixer => fixCode(fixer, context.getSourceCode(), node, memberExpression);
 			}
 
-			context.report(problem);
+			context.report({
+				node,
+				message,
+				data: {
+					replacement,
+					method: eventMethodName,
+					extra: extra ? ` ${extra}` : ''
+				},
+				fix
+			});
 		}
 	};
 };

diff --git a/test/prefer-add-event-listener.js b/test/prefer-add-event-listener.js
@@ -43,6 +43,11 @@ const expectedBeforeUnloadWithReturnMessage = [
 	'Use `event.preventDefault(); event.returnValue = \'foo\'` to trigger the prompt.'
 ].join(' ');
 
+const expectedMessageEventWithReturnMessage = [
+	'Prefer `addEventListener` over `onmessage`.',
+	'Note that there is difference between `SharedWorker#onmessage` and `SharedWorker#addEventListener(\'message\')`.'
+].join(' ');
+
 ruleTester.run('prefer-add-event-listener', rule, {
 	valid: [
 		'foo.addEventListener(\'click\', () => {})',
@@ -147,38 +152,38 @@ ruleTester.run('prefer-add-event-listener', rule, {
 		),
 		invalidTestCase(
 			'foo.onclick = null',
-			null,
-			null,
+			undefined,
+			undefined,
 			'Prefer `removeEventListener` over `onclick`.'
 		),
 		invalidTestCase(
 			'foo.onclick = undefined',
-			null,
-			null,
+			undefined,
+			undefined,
 			'Prefer `removeEventListener` over `onclick`.'
 		),
 		invalidTestCase(
 			'window.onbeforeunload = null',
-			null,
-			null,
+			undefined,
+			undefined,
 			'Prefer `removeEventListener` over `onbeforeunload`.'
 		),
 		invalidTestCase(
 			'window.onbeforeunload = undefined',
-			null,
-			null,
+			undefined,
+			undefined,
 			'Prefer `removeEventListener` over `onbeforeunload`.'
 		),
 		invalidTestCase(
 			'window.onbeforeunload = foo',
-			null,
-			null,
+			undefined,
+			undefined,
 			expectedBeforeUnloadWithReturnMessage
 		),
 		invalidTestCase(
 			'window.onbeforeunload = () => \'foo\'',
-			null,
-			null,
+			undefined,
+			undefined,
 			expectedBeforeUnloadWithReturnMessage
 		),
 		invalidTestCase(
@@ -187,8 +192,8 @@ ruleTester.run('prefer-add-event-listener', rule, {
 					return bar;
 				}
 			`,
-			null,
-			null,
+			undefined,
+			undefined,
 			expectedBeforeUnloadWithReturnMessage
 		),
 		invalidTestCase(
@@ -197,8 +202,8 @@ ruleTester.run('prefer-add-event-listener', rule, {
 					return 'bar';
 				}
 			`,
-			null,
-			null,
+			undefined,
+			undefined,
 			expectedBeforeUnloadWithReturnMessage
 		),
 		invalidTestCase(
@@ -338,6 +343,12 @@ ruleTester.run('prefer-add-event-listener', rule, {
 			`,
 			'onerror',
 			[{excludedPackages: []}]
+		),
+		invalidTestCase(
+			'myWorker.port.onmessage = function(e) {}',
+			undefined,
+			undefined,
+			expectedMessageEventWithReturnMessage
 		)
 	]
 });