theKashey / react-focus-lock

It is a trap! A lock for a Focus. 🔓
MIT License
1.25k stars 66 forks source link

Fix pure annotation warnings #275

Closed mrm007 closed 5 months ago

mrm007 commented 6 months ago

Fixes #273

I couldn't install dependencies locally — it failed because of some Atlassian packages in the lockfile (I solved it by removing them from the lockfile and running yarn install again)

Some things I tried:

The simplest solution is to add "generatorOpts": { "concise": true } (reference) to the Babel config. This makes the PURE annotation be on the same line, thus no longer triggering Rollup. Here's a snippet of the output:

Details ```js var mergedRef = useMergeRefs([parentRef, setObserveNode]); return /*#__PURE__*/React.createElement(React.Fragment, null, hasLeadingGuards && [ /*#__PURE__*/ // nearest focus guard React.createElement("div", { key: "guard-first", "data-focus-guard": true, tabIndex: disabled ? -1 : 0, style: hiddenGuard }), // first tabbed element guard hasPositiveIndices ? /*#__PURE__*/React.createElement("div", { // etc. ```

Edit: That didn't actually work. Using shouldPrintComment with a modified default works.

theKashey commented 6 months ago

This doesn't actually work.

So it works or does not? Probably the simplest way to resolve a problem is migrate this project to TypeScript, which is something I want to do every day.

it failed because of some Atlassian packages in the lockfile

Sorry, sometimes global settings has side effects

mrm007 commented 6 months ago

So it works or does not?

@theKashey the initial solution with concise: true didn't work, but this one with shouldPrintComment does.

mrm007 commented 5 months ago

Can we merge this, please? It's breaking some of our builds at SEEK.

Here's the diff of the build output with changes from this PR:

Details ```diff diff --color -bur dist_old/cjs/Combination.js dist_new/cjs/Combination.js --- dist_old/cjs/Combination.js 2024-01-23 12:36:00 +++ dist_new/cjs/Combination.js 2024-01-23 12:36:29 @@ -13,15 +13,6 @@ var _Trap = _interopRequireDefault(require("./Trap")); function _getRequireWildcardCache(e) { if ("function" != typeof WeakMap) return null; var r = new WeakMap(), t = new WeakMap(); return (_getRequireWildcardCache = function _getRequireWildcardCache(e) { return e ? t : r; })(e); } function _interopRequireWildcard(e, r) { if (!r && e && e.__esModule) return e; if (null === e || "object" != _typeof(e) && "function" != typeof e) return { "default": e }; var t = _getRequireWildcardCache(r); if (t && t.has(e)) return t.get(e); var n = { __proto__: null }, a = Object.defineProperty && Object.getOwnPropertyDescriptor; for (var u in e) { if ("default" !== u && Object.prototype.hasOwnProperty.call(e, u)) { var i = a ? Object.getOwnPropertyDescriptor(e, u) : null; i && (i.get || i.set) ? Object.defineProperty(n, u, i) : n[u] = e[u]; } } return n["default"] = e, t && t.set(e, n), n; } -/* that would be a BREAKING CHANGE! -// delaying sidecar execution till the first usage -const RequireSideCar = (props) => { - // eslint-disable-next-line global-require - const SideCar = require('./Trap').default; - return ; -}; -*/ - var FocusLockCombination = /*#__PURE__*/React.forwardRef(function FocusLockUICombination(props, ref) { return /*#__PURE__*/React.createElement(_Lock["default"], (0, _extends2["default"])({ sideCar: _Trap["default"], diff --color -bur dist_old/cjs/Lock.js dist_new/cjs/Lock.js --- dist_old/cjs/Lock.js 2024-01-23 12:36:00 +++ dist_new/cjs/Lock.js 2024-01-23 12:36:29 @@ -60,7 +60,7 @@ onDeactivationCallback = props.onDeactivation; var _React$useState3 = React.useState({}), _React$useState4 = (0, _slicedToArray2["default"])(_React$useState3, 1), - id = _React$useState4[0]; // SIDE EFFECT CALLBACKS + id = _React$useState4[0]; var onActivation = React.useCallback(function () { originalFocusedElement.current = originalFocusedElement.current || document && document.activeElement; if (observed.current && onActivationCallback) { @@ -76,8 +76,6 @@ }, [onDeactivationCallback]); (0, React.useEffect)(function () { if (!disabled) { - // cleanup return focus on trap deactivation - // sideEffect/returnFocus should happen by this time originalFocusedElement.current = null; } }, []); @@ -89,8 +87,6 @@ var returnFocusOptions = (0, _typeof2["default"])(howToReturnFocus) === 'object' ? howToReturnFocus : undefined; originalFocusedElement.current = null; if (allowDefer) { - // React might return focus after update - // it's safer to defer the action Promise.resolve().then(function () { return returnFocusTo.focus(returnFocusOptions); }); @@ -100,19 +96,12 @@ } } }, [shouldReturnFocus]); - - // MEDIUM CALLBACKS - var onFocus = React.useCallback(function (event) { if (isActive.current) { _medium.mediumFocus.useMedium(event); } }, []); var onBlur = _medium.mediumBlur.useMedium; - - // REF PROPAGATION - // not using real refs due to race conditions - var setObserveNode = React.useCallback(function (newObserved) { if (observed.current !== newObserved) { observed.current = newObserved; @@ -121,13 +110,10 @@ }, []); if (process.env.NODE_ENV !== 'production') { if (typeof allowTextSelection !== 'undefined') { - // eslint-disable-next-line no-console console.warn('React-Focus-Lock: allowTextSelection is deprecated and enabled by default'); } React.useEffect(function () { - // report incorrect integration - https://github.com/theKashey/react-focus-lock/issues/123 if (!observed.current && typeof Container !== 'string') { - // eslint-disable-next-line no-console console.error('FocusLock: could not obtain ref to internal node'); } }, []); @@ -138,15 +124,12 @@ var mergedRef = (0, _useCallbackRef.useMergeRefs)([parentRef, setObserveNode]); return /*#__PURE__*/React.createElement(React.Fragment, null, hasLeadingGuards && [ /*#__PURE__*/ - // nearest focus guard React.createElement("div", { key: "guard-first", "data-focus-guard": true, tabIndex: disabled ? -1 : 0, style: _FocusGuard.hiddenGuard - }), - // first tabbed element guard - hasPositiveIndices ? /*#__PURE__*/React.createElement("div", { + }), hasPositiveIndices ? /*#__PURE__*/React.createElement("div", { key: "guard-nearest", "data-focus-guard": true, tabIndex: disabled ? -1 : 1, diff --color -bur dist_old/cjs/Trap.js dist_new/cjs/Trap.js --- dist_old/cjs/Trap.js 2024-01-23 12:36:00 +++ dist_new/cjs/Trap.js 2024-01-23 12:36:29 @@ -15,8 +15,6 @@ var _medium = require("./medium"); function _getRequireWildcardCache(e) { if ("function" != typeof WeakMap) return null; var r = new WeakMap(), t = new WeakMap(); return (_getRequireWildcardCache = function _getRequireWildcardCache(e) { return e ? t : r; })(e); } function _interopRequireWildcard(e, r) { if (!r && e && e.__esModule) return e; if (null === e || "object" != _typeof(e) && "function" != typeof e) return { "default": e }; var t = _getRequireWildcardCache(r); if (t && t.has(e)) return t.get(e); var n = { __proto__: null }, a = Object.defineProperty && Object.getOwnPropertyDescriptor; for (var u in e) { if ("default" !== u && Object.prototype.hasOwnProperty.call(e, u)) { var i = a ? Object.getOwnPropertyDescriptor(e, u) : null; i && (i.get || i.set) ? Object.defineProperty(n, u, i) : n[u] = e[u]; } } return n["default"] = e, t && t.set(e, n), n; } -/* eslint-disable no-mixed-operators */ - var focusOnBody = function focusOnBody() { return document && document.activeElement === document.body; }; @@ -53,7 +51,6 @@ } } else if (item.lockItem) { if (i !== startIndex) { - // we will tab to the next element return; } lastGuard = null; @@ -70,18 +67,12 @@ }; var focusWasOutside = function focusWasOutside(crossFrameOption) { if (crossFrameOption) { - // with cross frame return true for any value return Boolean(focusWasOutsideWindow); } - // in other case return only of focus went a while aho return focusWasOutsideWindow === 'meanwhile'; }; var checkInHost = function checkInHost(check, el, boundary) { - return el && ( - // find host equal to active element and check nested active element - el.host === check && (!el.activeElement || boundary.contains(el.activeElement)) - // dive up - || el.parentNode && checkInHost(check, el.parentNode, boundary)); + return el && (el.host === check && (!el.activeElement || boundary.contains(el.activeElement)) || el.parentNode && checkInHost(check, el.parentNode, boundary)); }; var withinHost = function withinHost(activeElement, workingArea) { return workingArea.some(function (area) { @@ -104,13 +95,8 @@ var workingArea = [workingNode].concat((0, _toConsumableArray2["default"])(shards.map(extractRef).filter(Boolean))); if (!activeElement || focusWhitelisted(activeElement)) { if (persistentFocus || focusWasOutside(crossFrame) || !isFreeFocus() || !lastActiveFocus && autoFocus) { - if (workingNode && !( - // active element is "inside" working area - (0, _focusLock.focusInside)(workingArea) || - // check for shadow-dom contained elements - activeElement && withinHost(activeElement, workingArea) || focusIsPortaledPair(activeElement, workingNode))) { + if (workingNode && !((0, _focusLock.focusInside)(workingArea) || activeElement && withinHost(activeElement, workingArea) || focusIsPortaledPair(activeElement, workingNode))) { if (document && !lastActiveFocus && activeElement && !autoFocus) { - // Check if blur() exists, which is missing on certain elements on IE if (activeElement.blur) { activeElement.blur(); } @@ -134,7 +120,6 @@ return node; }).indexOf(newActiveElement); if (focusedIndex > -1) { - // remove old focus allNodes.filter(function (_ref2) { var guard = _ref2.guard, node = _ref2.node; @@ -153,7 +138,6 @@ }; var onTrap = function onTrap(event) { if (activateTrap() && event) { - // prevent scroll jump event.stopPropagation(); event.preventDefault(); } @@ -162,7 +146,6 @@ return (0, _util.deferAction)(activateTrap); }; var onFocus = function onFocus(event) { - // detect portal var source = event.target; var currentNode = event.currentTarget; if (!currentNode.contains(source)) { @@ -184,7 +167,6 @@ } : {}; var onWindowBlur = function onWindowBlur() { focusWasOutsideWindow = 'just'; - // using setTimeout to set this variable after React/sidecar reaction (0, _util.deferAction)(function () { focusWasOutsideWindow = 'meanwhile'; }); @@ -215,12 +197,10 @@ lastActiveTrap = trap; if (lastTrap && !sameTrap) { lastTrap.onDeactivation(); - // return focus only of last trap was removed if (!traps.filter(function (_ref6) { var id = _ref6.id; return id === lastTrap.id; }).length) { - // allow defer is no other trap is awaiting restore lastTrap.returnFocus(!trap); } } @@ -236,8 +216,6 @@ lastActiveFocus = null; } } - -// bind medium _medium.mediumFocus.assignSyncMedium(onFocus); _medium.mediumBlur.assignMedium(onBlur); _medium.mediumEffect.assignMedium(function (cb) { diff --color -bur dist_old/cjs/clientSideEffect.js dist_new/cjs/clientSideEffect.js --- dist_old/cjs/clientSideEffect.js 2024-01-23 12:36:00 +++ dist_new/cjs/clientSideEffect.js 2024-01-23 12:36:29 @@ -8,10 +8,6 @@ var React = _interopRequireWildcard(require("react")); function _getRequireWildcardCache(e) { if ("function" != typeof WeakMap) return null; var r = new WeakMap(), t = new WeakMap(); return (_getRequireWildcardCache = function _getRequireWildcardCache(e) { return e ? t : r; })(e); } function _interopRequireWildcard(e, r) { if (!r && e && e.__esModule) return e; if (null === e || "object" != _typeof(e) && "function" != typeof e) return { "default": e }; var t = _getRequireWildcardCache(r); if (t && t.has(e)) return t.get(e); var n = { __proto__: null }, a = Object.defineProperty && Object.getOwnPropertyDescriptor; for (var u in e) { if ("default" !== u && Object.prototype.hasOwnProperty.call(e, u)) { var i = a ? Object.getOwnPropertyDescriptor(e, u) : null; i && (i.get || i.set) ? Object.defineProperty(n, u, i) : n[u] = e[u]; } } return n["default"] = e, t && t.set(e, n), n; } -/* eslint-disable */ - -// NOT USED - function withSideEffect(reducePropsToState, handleStateChangeOnClient) { if (process.env.NODE_ENV !== 'production') { if (typeof reducePropsToState !== 'function') { @@ -40,8 +36,6 @@ React.useEffect(function () { lastProps.current = props; }); - - // handle mounted instances React.useEffect(function () { console.log('ins added'); mountedInstances.push(lastProps); @@ -51,10 +45,6 @@ mountedInstances.splice(index, 1); }; }, []); - - // notify updates - // React.useEffect(emitChange, [props.disabled]); - return /*#__PURE__*/React.createElement(WrappedComponent, props); }; return SideEffect; diff --color -bur dist_old/cjs/medium.js dist_new/cjs/medium.js --- dist_old/cjs/medium.js 2024-01-23 12:36:00 +++ dist_new/cjs/medium.js 2024-01-23 12:36:29 @@ -20,8 +20,5 @@ exports.mediumEffect = mediumEffect; var mediumSidecar = (0, _useSidecar.createSidecarMedium)({ async: true - // focus-lock sidecar is not required on the server - // however, it might be required for JSDOM tests - // ssr: true, }); exports.mediumSidecar = mediumSidecar; \ No newline at end of file diff --color -bur dist_old/es2015/Combination.js dist_new/es2015/Combination.js --- dist_old/es2015/Combination.js 2024-01-23 12:35:59 +++ dist_new/es2015/Combination.js 2024-01-23 12:36:28 @@ -3,16 +3,6 @@ import * as React from 'react'; import FocusLockUI from './Lock'; import FocusTrap from './Trap'; - -/* that would be a BREAKING CHANGE! -// delaying sidecar execution till the first usage -const RequireSideCar = (props) => { - // eslint-disable-next-line global-require - const SideCar = require('./Trap').default; - return ; -}; -*/ - var FocusLockCombination = /*#__PURE__*/React.forwardRef(function FocusLockUICombination(props, ref) { return /*#__PURE__*/React.createElement(FocusLockUI, _extends({ sideCar: FocusTrap, diff --color -bur dist_old/es2015/Lock.js dist_new/es2015/Lock.js --- dist_old/es2015/Lock.js 2024-01-23 12:35:59 +++ dist_new/es2015/Lock.js 2024-01-23 12:36:28 @@ -44,7 +44,7 @@ onActivationCallback = props.onActivation, onDeactivationCallback = props.onDeactivation; var _React$useState2 = React.useState({}), - id = _React$useState2[0]; // SIDE EFFECT CALLBACKS + id = _React$useState2[0]; var onActivation = React.useCallback(function () { originalFocusedElement.current = originalFocusedElement.current || document && document.activeElement; if (observed.current && onActivationCallback) { @@ -60,8 +60,6 @@ }, [onDeactivationCallback]); useEffect(function () { if (!disabled) { - // cleanup return focus on trap deactivation - // sideEffect/returnFocus should happen by this time originalFocusedElement.current = null; } }, []); @@ -73,8 +71,6 @@ var returnFocusOptions = typeof howToReturnFocus === 'object' ? howToReturnFocus : undefined; originalFocusedElement.current = null; if (allowDefer) { - // React might return focus after update - // it's safer to defer the action Promise.resolve().then(function () { return returnFocusTo.focus(returnFocusOptions); }); @@ -84,19 +80,12 @@ } } }, [shouldReturnFocus]); - - // MEDIUM CALLBACKS - var onFocus = React.useCallback(function (event) { if (isActive.current) { mediumFocus.useMedium(event); } }, []); var onBlur = mediumBlur.useMedium; - - // REF PROPAGATION - // not using real refs due to race conditions - var setObserveNode = React.useCallback(function (newObserved) { if (observed.current !== newObserved) { observed.current = newObserved; @@ -105,13 +94,10 @@ }, []); if (process.env.NODE_ENV !== 'production') { if (typeof allowTextSelection !== 'undefined') { - // eslint-disable-next-line no-console console.warn('React-Focus-Lock: allowTextSelection is deprecated and enabled by default'); } React.useEffect(function () { - // report incorrect integration - https://github.com/theKashey/react-focus-lock/issues/123 if (!observed.current && typeof Container !== 'string') { - // eslint-disable-next-line no-console console.error('FocusLock: could not obtain ref to internal node'); } }, []); @@ -122,15 +108,12 @@ var mergedRef = useMergeRefs([parentRef, setObserveNode]); return /*#__PURE__*/React.createElement(React.Fragment, null, hasLeadingGuards && [ /*#__PURE__*/ - // nearest focus guard React.createElement("div", { key: "guard-first", "data-focus-guard": true, tabIndex: disabled ? -1 : 0, style: hiddenGuard - }), - // first tabbed element guard - hasPositiveIndices ? /*#__PURE__*/React.createElement("div", { + }), hasPositiveIndices ? /*#__PURE__*/React.createElement("div", { key: "guard-nearest", "data-focus-guard": true, tabIndex: disabled ? -1 : 1, diff --color -bur dist_old/es2015/Trap.js dist_new/es2015/Trap.js --- dist_old/es2015/Trap.js 2024-01-23 12:35:59 +++ dist_new/es2015/Trap.js 2024-01-23 12:36:28 @@ -1,4 +1,3 @@ -/* eslint-disable no-mixed-operators */ import * as React from 'react'; import PropTypes from 'prop-types'; import withSideEffect from 'react-clientside-effect'; @@ -41,7 +40,6 @@ } } else if (item.lockItem) { if (i !== startIndex) { - // we will tab to the next element return; } lastGuard = null; @@ -58,18 +56,12 @@ }; var focusWasOutside = function focusWasOutside(crossFrameOption) { if (crossFrameOption) { - // with cross frame return true for any value return Boolean(focusWasOutsideWindow); } - // in other case return only of focus went a while aho return focusWasOutsideWindow === 'meanwhile'; }; var checkInHost = function checkInHost(check, el, boundary) { - return el && ( - // find host equal to active element and check nested active element - el.host === check && (!el.activeElement || boundary.contains(el.activeElement)) - // dive up - || el.parentNode && checkInHost(check, el.parentNode, boundary)); + return el && (el.host === check && (!el.activeElement || boundary.contains(el.activeElement)) || el.parentNode && checkInHost(check, el.parentNode, boundary)); }; var withinHost = function withinHost(activeElement, workingArea) { return workingArea.some(function (area) { @@ -92,13 +84,8 @@ var workingArea = [workingNode].concat(shards.map(extractRef).filter(Boolean)); if (!activeElement || focusWhitelisted(activeElement)) { if (persistentFocus || focusWasOutside(crossFrame) || !isFreeFocus() || !lastActiveFocus && autoFocus) { - if (workingNode && !( - // active element is "inside" working area - focusInside(workingArea) || - // check for shadow-dom contained elements - activeElement && withinHost(activeElement, workingArea) || focusIsPortaledPair(activeElement, workingNode))) { + if (workingNode && !(focusInside(workingArea) || activeElement && withinHost(activeElement, workingArea) || focusIsPortaledPair(activeElement, workingNode))) { if (document && !lastActiveFocus && activeElement && !autoFocus) { - // Check if blur() exists, which is missing on certain elements on IE if (activeElement.blur) { activeElement.blur(); } @@ -122,7 +109,6 @@ return node; }).indexOf(newActiveElement); if (focusedIndex > -1) { - // remove old focus allNodes.filter(function (_ref2) { var guard = _ref2.guard, node = _ref2.node; @@ -141,7 +127,6 @@ }; var onTrap = function onTrap(event) { if (activateTrap() && event) { - // prevent scroll jump event.stopPropagation(); event.preventDefault(); } @@ -150,7 +135,6 @@ return deferAction(activateTrap); }; var onFocus = function onFocus(event) { - // detect portal var source = event.target; var currentNode = event.currentTarget; if (!currentNode.contains(source)) { @@ -172,7 +156,6 @@ } : {}; var onWindowBlur = function onWindowBlur() { focusWasOutsideWindow = 'just'; - // using setTimeout to set this variable after React/sidecar reaction deferAction(function () { focusWasOutsideWindow = 'meanwhile'; }); @@ -203,12 +186,10 @@ lastActiveTrap = trap; if (lastTrap && !sameTrap) { lastTrap.onDeactivation(); - // return focus only of last trap was removed if (!traps.filter(function (_ref6) { var id = _ref6.id; return id === lastTrap.id; }).length) { - // allow defer is no other trap is awaiting restore lastTrap.returnFocus(!trap); } } @@ -224,8 +205,6 @@ lastActiveFocus = null; } } - -// bind medium mediumFocus.assignSyncMedium(onFocus); mediumBlur.assignMedium(onBlur); mediumEffect.assignMedium(function (cb) { diff --color -bur dist_old/es2015/clientSideEffect.js dist_new/es2015/clientSideEffect.js --- dist_old/es2015/clientSideEffect.js 2024-01-23 12:35:59 +++ dist_new/es2015/clientSideEffect.js 2024-01-23 12:36:28 @@ -1,9 +1,4 @@ -/* eslint-disable */ - import * as React from 'react'; - -// NOT USED - function withSideEffect(reducePropsToState, handleStateChangeOnClient) { if (process.env.NODE_ENV !== 'production') { if (typeof reducePropsToState !== 'function') { @@ -32,8 +27,6 @@ React.useEffect(function () { lastProps.current = props; }); - - // handle mounted instances React.useEffect(function () { console.log('ins added'); mountedInstances.push(lastProps); @@ -43,10 +36,6 @@ mountedInstances.splice(index, 1); }; }, []); - - // notify updates - // React.useEffect(emitChange, [props.disabled]); - return /*#__PURE__*/React.createElement(WrappedComponent, props); }; return SideEffect; diff --color -bur dist_old/es2015/medium.js dist_new/es2015/medium.js --- dist_old/es2015/medium.js 2024-01-23 12:35:59 +++ dist_new/es2015/medium.js 2024-01-23 12:36:28 @@ -11,7 +11,4 @@ export var mediumEffect = createMedium(); export var mediumSidecar = createSidecarMedium({ async: true - // focus-lock sidecar is not required on the server - // however, it might be required for JSDOM tests - // ssr: true, }); \ No newline at end of file ```
theKashey commented 5 months ago

👋 @mrm007, sorry for being slow - work work work.

Looking for the diff, look like that's it

   /*#__PURE__*/
-  // nearest focus guard   <--- bad guy
   React.createElement("div", {
theKashey commented 5 months ago

v2.9.7 released

mrm007 commented 5 months ago

Thank you @theKashey 🙇‍♂️