Add eslint rule to warn against using globals for addEventListener

packages/block-library/src/navigation-link/edit.js

@@ -64,6 +64,8 @@ const useIsDraggingWithin = ( elementRef ) => {
 	const [ isDraggingWithin, setIsDraggingWithin ] = useState( false );
 
 	useEffect( () => {
+		const { ownerDocument } = elementRef.current;
+
 		function handleDragStart( event ) {
 			// Check the first time when the dragging starts.
 			handleDragEnter( event );
@@ -84,15 +86,16 @@ const useIsDraggingWithin = ( elementRef ) => {
 		}
 
 		// Bind these events to the document to catch all drag events.
-		// Ideally, we can also use `event.relatedTarget`, but sadly that doesn't work in Safari.
-		document.addEventListener( 'dragstart', handleDragStart );
-		document.addEventListener( 'dragend', handleDragEnd );
-		document.addEventListener( 'dragenter', handleDragEnter );
+		// Ideally, we can also use `event.relatedTarget`, but sadly that
+		// doesn't work in Safari.
+		ownerDocument.addEventListener( 'dragstart', handleDragStart );
+		ownerDocument.addEventListener( 'dragend', handleDragEnd );
+		ownerDocument.addEventListener( 'dragenter', handleDragEnter );
 
 		return () => {
-			document.removeEventListener( 'dragstart', handleDragStart );
-			document.removeEventListener( 'dragend', handleDragEnd );
-			document.removeEventListener( 'dragenter', handleDragEnter );
+			ownerDocument.removeEventListener( 'dragstart', handleDragStart );
+			ownerDocument.removeEventListener( 'dragend', handleDragEnd );
+			ownerDocument.removeEventListener( 'dragenter', handleDragEnter );
 		};
 	}, [] );
 

packages/components/src/sandbox/index.js

@@ -209,16 +209,19 @@ export default function Sandbox( {
 			setHeight( data.height );
 		}
 
+		const { ownerDocument } = ref.current;
+		const { defaultView } = ownerDocument;
+
 		// This used to be registered using <iframe onLoad={} />, but it made the iframe blank
 		// after reordering the containing block. See these two issues for more details:
 		// https://github.com/WordPress/gutenberg/issues/6146
 		// https://github.com/facebook/react/issues/18752
 		ref.current.addEventListener( 'load', tryNoForceSandbox, false );
-		window.addEventListener( 'message', checkMessageForResize );
+		defaultView.addEventListener( 'message', checkMessageForResize );
 
 		return () => {
 			ref.current.removeEventListener( 'load', tryNoForceSandbox, false );
-			window.addEventListener( 'message', checkMessageForResize );
+			defaultView.addEventListener( 'message', checkMessageForResize );
 		};
 	}, [] );
 

packages/edit-widgets/src/blocks/widget-area/edit/index.js

@@ -16,6 +16,8 @@ import { Panel, PanelBody } from '@wordpress/components';
  */
 import WidgetAreaInnerBlocks from './inner-blocks';
 
+/** @typedef {import('@wordpress/element').RefObject} RefObject */
+
 export default function WidgetAreaEdit( {
 	clientId,
 	className,
@@ -33,7 +35,7 @@ export default function WidgetAreaEdit( {
 		( openState ) => setIsWidgetAreaOpen( clientId, openState ),
 		[ clientId ]
 	);
-	const isDragging = useIsDragging();
+	const isDragging = useIsDragging( wrapper );
 	const isDraggingWithin = useIsDraggingWithin( wrapper );
 
 	const [ openedWhileDragging, setOpenedWhileDragging ] = useState( false );
@@ -82,14 +84,16 @@ export default function WidgetAreaEdit( {
 /**
  * A React hook to determine if dragging is active.
  *
- * @typedef {import('@wordpress/element').RefObject} RefObject
+ * @param {RefObject<HTMLElement>} elementRef The target elementRef object.
  *
  * @return {boolean} Is dragging within the entire document.
  */
-const useIsDragging = () => {
+const useIsDragging = ( elementRef ) => {
 	const [ isDragging, setIsDragging ] = useState( false );
 
 	useEffect( () => {
+		const { ownerDocument } = elementRef.current;
+
 		function handleDragStart() {
 			setIsDragging( true );
 		}
@@ -98,12 +102,12 @@ const useIsDragging = () => {
 			setIsDragging( false );
 		}
 
-		document.addEventListener( 'dragstart', handleDragStart );
-		document.addEventListener( 'dragend', handleDragEnd );
+		ownerDocument.addEventListener( 'dragstart', handleDragStart );
+		ownerDocument.addEventListener( 'dragend', handleDragEnd );
 
 		return () => {
-			document.removeEventListener( 'dragstart', handleDragStart );
-			document.removeEventListener( 'dragend', handleDragEnd );
+			ownerDocument.removeEventListener( 'dragstart', handleDragStart );
+			ownerDocument.removeEventListener( 'dragend', handleDragEnd );
 		};
 	}, [] );
 
@@ -113,8 +117,6 @@ const useIsDragging = () => {
 /**
  * A React hook to determine if it's dragging within the target element.
  *
- * @typedef {import('@wordpress/element').RefObject} RefObject
- *
  * @param {RefObject<HTMLElement>} elementRef The target elementRef object.
  *
  * @return {boolean} Is dragging within the target element.
@@ -123,6 +125,8 @@ const useIsDraggingWithin = ( elementRef ) => {
 	const [ isDraggingWithin, setIsDraggingWithin ] = useState( false );
 
 	useEffect( () => {
+		const { ownerDocument } = elementRef.current;
+
 		function handleDragStart( event ) {
 			// Check the first time when the dragging starts.
 			handleDragEnter( event );
@@ -144,14 +148,14 @@ const useIsDraggingWithin = ( elementRef ) => {
 
 		// Bind these events to the document to catch all drag events.
 		// Ideally, we can also use `event.relatedTarget`, but sadly that doesn't work in Safari.
-		document.addEventListener( 'dragstart', handleDragStart );
-		document.addEventListener( 'dragend', handleDragEnd );
-		document.addEventListener( 'dragenter', handleDragEnter );
+		ownerDocument.addEventListener( 'dragstart', handleDragStart );
+		ownerDocument.addEventListener( 'dragend', handleDragEnd );
+		ownerDocument.addEventListener( 'dragenter', handleDragEnter );
 
 		return () => {
-			document.removeEventListener( 'dragstart', handleDragStart );
-			document.removeEventListener( 'dragend', handleDragEnd );
-			document.removeEventListener( 'dragenter', handleDragEnter );
+			ownerDocument.removeEventListener( 'dragstart', handleDragStart );
+			ownerDocument.removeEventListener( 'dragend', handleDragEnd );
+			ownerDocument.removeEventListener( 'dragenter', handleDragEnter );
 		};
 	}, [] );
 

packages/eslint-plugin/configs/custom.js

@@ -6,6 +6,7 @@ module.exports = {
 		'@wordpress/no-unguarded-get-range-at': 'error',
 		'@wordpress/no-global-active-element': 'warn',
 		'@wordpress/no-global-get-selection': 'error',
+		'@wordpress/no-global-event-listener': 'warn',
 	},
 	overrides: [
 		{
@@ -23,6 +24,7 @@ module.exports = {
 			rules: {
 				'@wordpress/no-global-active-element': 'off',
 				'@wordpress/no-global-get-selection': 'off',
+				'@wordpress/no-global-event-listener': 'off',
 			},
 		},
 	],

packages/eslint-plugin/rules/__tests__/no-global-event-listener.js

@@ -0,0 +1,70 @@
+/**
+ * External dependencies
+ */
+import { RuleTester } from 'eslint';
+
+/**
+ * Internal dependencies
+ */
+import rule from '../no-global-event-listener';
+
+const ruleTester = new RuleTester( {
+	parserOptions: {
+		ecmaVersion: 6,
+	},
+} );
+
+ruleTester.run( 'no-global-event-listener', rule, {
+	valid: [
+		{
+			code: 'ownerDocument.addEventListener();',
+		},
+		{
+			code: 'ownerDocument.removeEventListener();',
+		},
+		{
+			code: 'defaultView.addEventListener();',
+		},
+		{
+			code: 'defaultView.removeEventListener();',
+		},
+	],
+	invalid: [
+		{
+			code: 'document.addEventListener();',
+			errors: [
+				{
+					message:
+						'Avoid using (add|remove)EventListener with globals. Use `ownerDocument` or `ownerDocument.defaultView` on a node ref instead.',
+				},
+			],
+		},
+		{
+			code: 'document.removeEventListener();',
+			errors: [
+				{
+					message:
+						'Avoid using (add|remove)EventListener with globals. Use `ownerDocument` or `ownerDocument.defaultView` on a node ref instead.',
+				},
+			],
+		},
+		{
+			code: 'window.addEventListener();',
+			errors: [
+				{
+					message:
+						'Avoid using (add|remove)EventListener with globals. Use `ownerDocument` or `ownerDocument.defaultView` on a node ref instead.',
+				},
+			],
+		},
+		{
+			code: 'window.removeEventListener();',
+			errors: [
+				{
+					message:
+						'Avoid using (add|remove)EventListener with globals. Use `ownerDocument` or `ownerDocument.defaultView` on a node ref instead.',
+				},
+			],
+		},
+	],
+} );

packages/eslint-plugin/rules/no-global-event-listener.js

@@ -0,0 +1,35 @@
+module.exports = {
+	meta: {
+		type: 'problem',
+		schema: [],
+	},
+	create( context ) {
+		return {
+			CallExpression( node ) {
+				const { callee } = node;
+				const { object, property } = callee;
+
+				if ( ! object || ! property ) {
+					return;
+				}
+
+				if ( object.name !== 'document' && object.name !== 'window' ) {
+					return;
+				}
+
+				if (
+					property.name !== 'addEventListener' &&
+					property.name !== 'removeEventListener'
+				) {
+					return;
+				}
+
+				context.report( {
+					node,
+					message:
+						'Avoid using (add|remove)EventListener with globals. Use `ownerDocument` or `ownerDocument.defaultView` on a node ref instead.',
+				} );
+			},
+		};
+	},
+};