From 351bb6fe23d1934e46b528ddc8a4837a25682c72 Mon Sep 17 00:00:00 2001 From: Andrew Musgrave Date: Mon, 3 Jun 2019 16:35:30 -0400 Subject: [PATCH 1/2] Remove componentWillReceiveProps --- .eslintignore | 1 + .../PositionedOverlay/PositionedOverlay.tsx | 55 +- tests/setup.ts | 1 + tests/shims/mutation-observer.ts | 723 ++++++++++++++++++ 4 files changed, 759 insertions(+), 21 deletions(-) create mode 100644 tests/shims/mutation-observer.ts diff --git a/.eslintignore b/.eslintignore index 18c8357154c..1a2f181b6d0 100644 --- a/.eslintignore +++ b/.eslintignore @@ -9,3 +9,4 @@ node_modules /styles.scss /styles /src/styles/polaris-tokens +tests/shims/mutation-observer.ts diff --git a/src/components/PositionedOverlay/PositionedOverlay.tsx b/src/components/PositionedOverlay/PositionedOverlay.tsx index a7e69ac4a20..88a446d1356 100644 --- a/src/components/PositionedOverlay/PositionedOverlay.tsx +++ b/src/components/PositionedOverlay/PositionedOverlay.tsx @@ -52,6 +52,8 @@ export interface State { lockPosition: boolean; } +const OBSERVER_CONFIG = {childList: true, subtree: true}; + export default class PositionedOverlay extends React.PureComponent< Props, State @@ -71,6 +73,13 @@ export default class PositionedOverlay extends React.PureComponent< private overlay: HTMLElement | null = null; private scrollableContainer: HTMLElement | Document | null = null; + private observer: MutationObserver; + + constructor(props: Props) { + super(props); + + this.observer = new MutationObserver(this.handleMeasurement); + } componentDidMount() { this.scrollableContainer = Scrollable.forNode(this.props.activator); @@ -92,10 +101,6 @@ export default class PositionedOverlay extends React.PureComponent< } } - componentWillReceiveProps() { - this.handleMeasurement(); - } - componentDidUpdate() { const {outsideScrollableContainer, top} = this.state; const {onScrollOut, active} = this.props; @@ -153,6 +158,8 @@ export default class PositionedOverlay extends React.PureComponent< private handleMeasurement = () => { const {lockPosition, top} = this.state; + this.observer.disconnect(); + this.setState( ({left, top}) => ({ left, @@ -220,23 +227,29 @@ export default class PositionedOverlay extends React.PureComponent< preferredAlignment, ); - this.setState({ - measuring: false, - activatorRect: getRectForNode(activator), - left: horizontalPosition, - top: lockPosition ? top : verticalPosition.top, - lockPosition: Boolean(fixed), - height: verticalPosition.height || 0, - width: fullWidth ? overlayRect.width : null, - positioning: verticalPosition.positioning as Positioning, - outsideScrollableContainer: - onScrollOut != null && - rectIsOutsideOfRect( - activatorRect, - intersectionWithViewport(scrollableContainerRect), - ), - zIndex, - }); + this.setState( + { + measuring: false, + activatorRect: getRectForNode(activator), + left: horizontalPosition, + top: lockPosition ? top : verticalPosition.top, + lockPosition: Boolean(fixed), + height: verticalPosition.height || 0, + width: fullWidth ? overlayRect.width : null, + positioning: verticalPosition.positioning as Positioning, + outsideScrollableContainer: + onScrollOut != null && + rectIsOutsideOfRect( + activatorRect, + intersectionWithViewport(scrollableContainerRect), + ), + zIndex, + }, + () => { + if (!this.overlay) return; + this.observer.observe(this.overlay, OBSERVER_CONFIG); + }, + ); }, ); }; diff --git a/tests/setup.ts b/tests/setup.ts index 5836457e643..9d83e8f7222 100644 --- a/tests/setup.ts +++ b/tests/setup.ts @@ -1,5 +1,6 @@ import {configure} from 'enzyme'; import Adapter from 'enzyme-adapter-react-16'; +import './shims/mutation-observer'; configure({adapter: new Adapter()}); diff --git a/tests/shims/mutation-observer.ts b/tests/shims/mutation-observer.ts new file mode 100644 index 00000000000..47545e9a322 --- /dev/null +++ b/tests/shims/mutation-observer.ts @@ -0,0 +1,723 @@ +/* ! + * Shim for MutationObserver interface + * Author: Graeme Yeates (github.com/megawac) + * Repository: https://github.com/megawac/MutationObserver.js + * License: WTFPL V2, 2004 (wtfpl.net). + * Though credit and staring the repo will make me feel pretty, you can modify and redistribute as you please. + * Attempts to follow spec (https://www.w3.org/TR/dom/#mutation-observers) as closely as possible for native javascript + * See https://github.com/WebKit/webkit/blob/master/Source/WebCore/dom/MutationObserver.cpp for current webkit source c++ implementation + */ + +/** + * prefix bugs: + - https://bugs.webkit.org/show_bug.cgi?id=85161 + - https://bugzilla.mozilla.org/show_bug.cgi?id=749920 + * Don't use WebKitMutationObserver as Safari (6.0.5-6.1) use a buggy implementation +*/ +(global as any).MutationObserver = + (global as any).MutationObserver || + (function(undefined) { + 'use strict'; + + /** + * @param {function(Array., MutationObserver)} listener + * @constructor + */ + function MutationObserver(listener) { + /** + * @type {Array.} + * @private + */ + this._watched = []; + /** @private */ + this._listener = listener; + } + + /** + * Start a recursive timeout function to check all items being observed for mutations + * @type {MutationObserver} observer + * @private + */ + function startMutationChecker(observer) { + (function check() { + const mutations = observer.takeRecords(); + + if (mutations.length) { + // fire away + // calling the listener with context is not spec but currently consistent with FF and WebKit + observer._listener(mutations, observer); + } + /** @private */ + observer._timeout = setTimeout(check, MutationObserver._period); + })(); + } + + /** + * Period to check for mutations (~32 times/sec) + * @type {number} + * @expose + */ + MutationObserver._period = 30 /* ms+runtime*/; + + /** + * Exposed API + * @expose + * @final + */ + MutationObserver.prototype = { + /** + * see https://dom.spec.whatwg.org/#dom-mutationobserver-observe + * not going to throw here but going to follow the current spec config sets + * @param {Node|null} $target + * @param {Object|null} config : MutationObserverInit configuration dictionary + * @expose + * @return undefined + */ + observe($target, config) { + /** + * Using slightly different names so closure can go ham + * @type {!Object} : A custom mutation config + */ + const settings: any = { + attr: Boolean( + config.attributes || + config.attributeFilter || + config.attributeOldValue, + ), + + // some browsers enforce that subtree must be set with childList, attributes or characterData. + // We don't care as spec doesn't specify this rule. + kids: Boolean(config.childList), + descendents: Boolean(config.subtree), + charData: Boolean( + config.characterData || config.characterDataOldValue, + ), + }; + + const watched = this._watched; + + // remove already observed target element from pool + for (let i = 0; i < watched.length; i++) { + if (watched[i].tar === $target) watched.splice(i, 1); + } + + if (config.attributeFilter) { + /** + * converts to a {key: true} dict for faster lookup + * @type {Object.} + */ + settings.afilter = reduce( + config.attributeFilter, + function(a, b) { + a[b] = true; + return a; + }, + {}, + ); + } + + watched.push({ + tar: $target, + fn: createMutationSearcher($target, settings), + }); + + // reconnect if not connected + if (!this._timeout) { + startMutationChecker(this); + } + }, + + /** + * Finds mutations since last check and empties the "record queue" i.e. mutations will only be found once + * @expose + * @return {Array.} + */ + takeRecords() { + const mutations = []; + const watched = this._watched; + + for (let i = 0; i < watched.length; i++) { + watched[i].fn(mutations); + } + + return mutations; + }, + + /** + * @expose + * @return undefined + */ + disconnect() { + this._watched = []; // clear the stuff being observed + clearTimeout(this._timeout); // ready for garbage collection + /** @private */ + this._timeout = null; + }, + }; + + /** + * Simple MutationRecord pseudoclass. No longer exposing as its not fully compliant + * @param {Object} data + * @return {Object} a MutationRecord + */ + function MutationRecord(data) { + const settings = { + // technically these should be on proto so hasOwnProperty will return false for non explicitly props + type: null, + target: null, + addedNodes: [], + removedNodes: [], + previousSibling: null, + nextSibling: null, + attributeName: null, + attributeNamespace: null, + oldValue: null, + }; + for (const prop in data) { + if (has(settings, prop) && data[prop] !== undefined) + settings[prop] = data[prop]; + } + return settings; + } + + /** + * Creates a func to find all the mutations + * + * @param {Node} $target + * @param {!Object} config : A custom mutation config + */ + function createMutationSearcher($target, config) { + /** type {Elestuct} */ + let $oldstate: any = clone($target, config); // create the cloned datastructure + + /** + * consumes array of mutations we can push to + * + * @param {Array.} mutations + */ + return function(mutations) { + const olen = mutations.length; + let dirty; + + if ( + config.charData && + $target.nodeType === 3 && + $target.nodeValue !== $oldstate.charData + ) { + mutations.push( + new (MutationRecord as MutationRecord & any)({ + type: 'characterData', + target: $target, + oldValue: $oldstate.charData, + }), + ); + } + + // Alright we check base level changes in attributes... easy + if (config.attr && $oldstate.attr) { + findAttributeMutations( + mutations, + $target, + $oldstate.attr, + config.afilter, + ); + } + + // check childlist or subtree for mutations + if (config.kids || config.descendents) { + dirty = searchSubtree(mutations, $target, $oldstate, config); + } + + // reclone data structure if theres changes + if (dirty || mutations.length !== olen) { + /** type {Elestuct} */ + $oldstate = clone($target, config); + } + }; + } + + /* attributes + attributeFilter helpers */ + + // Check if the environment has the attribute bug (#4) which cause + // element.attributes.style to always be null. + let hasAttributeBug: HTMLElement | boolean = document.createElement('i'); + hasAttributeBug.style.top = (0 as unknown) as string; + hasAttributeBug = (hasAttributeBug.attributes as any).style.value != 'null'; + + /** + * Gets an attribute value in an environment without attribute bug + * + * @param {Node} el + * @param {Attr} attr + * @return {String} an attribute value + */ + function getAttributeSimple(el, attr) { + // There is a potential for a warning to occur here if the attribute is a + // custom attribute in IE<9 with a custom .toString() method. This is + // just a warning and doesn't affect execution (see #21) + return attr.value; + } + + /** + * Gets an attribute value with special hack for style attribute (see #4) + * + * @param {Node} el + * @param {Attr} attr + * @return {String} an attribute value + */ + function getAttributeWithStyleHack(el, attr) { + // As with getAttributeSimple there is a potential warning for custom attribtues in IE7. + return attr.name !== 'style' ? attr.value : el.style.cssText; + } + + const getAttributeValue = hasAttributeBug + ? getAttributeSimple + : getAttributeWithStyleHack; + + /** + * fast helper to check to see if attributes object of an element has changed + * doesnt handle the textnode case + * + * @param {Array.} mutations + * @param {Node} $target + * @param {Object.} $oldstate : Custom attribute clone data structure from clone + * @param {Object} filter + */ + function findAttributeMutations(mutations, $target, $oldstate, filter) { + const checked = {}; + const attributes = $target.attributes; + let attr; + let name; + let i = attributes.length; + while (i--) { + attr = attributes[i]; + name = attr.name; + if (!filter || has(filter, name)) { + if (getAttributeValue($target, attr) !== $oldstate[name]) { + // The pushing is redundant but gzips very nicely + mutations.push( + MutationRecord({ + type: 'attributes', + target: $target, + attributeName: name, + oldValue: $oldstate[name], + attributeNamespace: attr.namespaceURI, // in ie<8 it incorrectly will return undefined + }), + ); + } + checked[name] = true; + } + } + for (name in $oldstate) { + if (!checked[name]) { + mutations.push( + MutationRecord({ + target: $target, + type: 'attributes', + attributeName: name, + oldValue: $oldstate[name], + }), + ); + } + } + } + + /** + * searchSubtree: array of mutations so far, element, element clone, bool + * synchronous dfs comparision of two nodes + * This function is applied to any observed element with childList or subtree specified + * Sorry this is kind of confusing as shit, tried to comment it a bit... + * codereview.stackexchange.com/questions/38351 discussion of an earlier version of this func + * + * @param {Array} mutations + * @param {Node} $target + * @param {!Object} $oldstate : A custom cloned node from clone() + * @param {!Object} config : A custom mutation config + */ + function searchSubtree(mutations, $target, $oldstate, config) { + // Track if the tree is dirty and has to be recomputed (#14). + let dirty; + /* + * Helper to identify node rearrangment and stuff... + * There is no gaurentee that the same node will be identified for both added and removed nodes + * if the positions have been shuffled. + * conflicts array will be emptied by end of operation + */ + function resolveConflicts( + conflicts, + node, + $kids, + $oldkids, + numAddedNodes, + ) { + // the distance between the first conflicting node and the last + const distance = conflicts.length - 1; + // prevents same conflict being resolved twice consider when two nodes switch places. + // only one should be given a mutation event (note -~ is used as a math.ceil shorthand) + let counter = -~((distance - numAddedNodes) / 2); + let $cur; + let oldstruct; + let conflict; + while ((conflict = conflicts.pop())) { + $cur = $kids[conflict.i]; + oldstruct = $oldkids[conflict.j]; + + // attempt to determine if there was node rearrangement... won't gaurentee all matches + // also handles case where added/removed nodes cause nodes to be identified as conflicts + if ( + config.kids && + counter && + Math.abs(conflict.i - conflict.j) >= distance + ) { + mutations.push( + MutationRecord({ + type: 'childList', + target: node, + addedNodes: [$cur], + removedNodes: [$cur], + // haha don't rely on this please + nextSibling: $cur.nextSibling, + previousSibling: $cur.previousSibling, + }), + ); + counter--; // found conflict + } + + // Alright we found the resorted nodes now check for other types of mutations + if (config.attr && oldstruct.attr) + findAttributeMutations( + mutations, + $cur, + oldstruct.attr, + config.afilter, + ); + if ( + config.charData && + $cur.nodeType === 3 && + $cur.nodeValue !== oldstruct.charData + ) { + mutations.push( + MutationRecord({ + type: 'characterData', + target: $cur, + oldValue: oldstruct.charData, + }), + ); + } + // now look @ subtree + if (config.descendents) findMutations($cur, oldstruct); + } + } + + /** + * Main worker. Finds and adds mutations if there are any + * @param {Node} node + * @param {!Object} old : A cloned data structure using internal clone + */ + function findMutations(node, old) { + const $kids = node.childNodes; + const $oldkids = old.kids; + const klen = $kids.length; + // $oldkids will be undefined for text and comment nodes + const olen = $oldkids ? $oldkids.length : 0; + // if (!olen && !klen) return; // both empty; clearly no changes + + // we delay the intialization of these for marginal performance in the expected case (actually quite signficant on large subtrees when these would be otherwise unused) + // map of checked element of ids to prevent registering the same conflict twice + let map; + // array of potential conflicts (ie nodes that may have been re arranged) + let conflicts; + let id; // element id from getElementId helper + let idx; // index of a moved or inserted element + + let oldstruct; + // current and old nodes + let $cur; + let $old; + // track the number of added nodes so we can resolve conflicts more accurately + let numAddedNodes = 0; + + // iterate over both old and current child nodes at the same time + let i = 0; + let j = 0; + // while there is still anything left in $kids or $oldkids (same as i < $kids.length || j < $oldkids.length;) + while (i < klen || j < olen) { + // current and old nodes at the indexs + $cur = $kids[i]; + oldstruct = $oldkids[j]; + $old = oldstruct && oldstruct.node; + + if ($cur === $old) { + // expected case - optimized for this case + // check attributes as specified by config + if (config.attr && oldstruct.attr) + /* oldstruct.attr instead of textnode check */ findAttributeMutations( + mutations, + $cur, + oldstruct.attr, + config.afilter, + ); + // check character data if node is a comment or textNode and it's being observed + if ( + config.charData && + oldstruct.charData !== undefined && + $cur.nodeValue !== oldstruct.charData + ) { + mutations.push( + MutationRecord({ + type: 'characterData', + target: $cur, + oldValue: oldstruct.charData, + }), + ); + } + + // resolve conflicts; it will be undefined if there are no conflicts - otherwise an array + if (conflicts) + resolveConflicts(conflicts, node, $kids, $oldkids, numAddedNodes); + + // recurse on next level of children. Avoids the recursive call when there are no children left to iterate + if ( + config.descendents && + ($cur.childNodes.length || + (oldstruct.kids && oldstruct.kids.length)) + ) + findMutations($cur, oldstruct); + + i++; + j++; + } else { + // (uncommon case) lookahead until they are the same again or the end of children + dirty = true; + if (!map) { + // delayed initalization (big perf benefit) + map = {}; + conflicts = []; + } + if ($cur) { + // check id is in the location map otherwise do a indexOf search + if (!map[(id = getElementId($cur))]) { + // to prevent double checking + // mark id as found + map[id] = true; + // custom indexOf using comparitor checking oldkids[i].node === $cur + if ((idx = indexOfCustomNode($oldkids, $cur, j)) === -1) { + if (config.kids) { + mutations.push( + MutationRecord({ + type: 'childList', + target: node, + addedNodes: [$cur], // $cur is a new node + nextSibling: $cur.nextSibling, + previousSibling: $cur.previousSibling, + }), + ); + numAddedNodes++; + } + } else { + conflicts.push({ + // add conflict + i, + j: idx, + }); + } + } + i++; + } + + if ( + $old && + // special case: the changes may have been resolved: i and j appear congurent so we can continue using the expected case + $old !== $kids[i] + ) { + if (!map[(id = getElementId($old))]) { + map[id] = true; + if ((idx = indexOf($kids, $old, i, _)) === -1) { + if (config.kids) { + mutations.push( + MutationRecord({ + type: 'childList', + target: old.node, + removedNodes: [$old], + nextSibling: $oldkids[j + 1], // praise no indexoutofbounds exception + previousSibling: $oldkids[j - 1], + }), + ); + numAddedNodes--; + } + } else { + conflicts.push({ + i: idx, + j, + }); + } + } + j++; + } + } // end uncommon case + } // end loop + + // resolve any remaining conflicts + if (conflicts) + resolveConflicts(conflicts, node, $kids, $oldkids, numAddedNodes); + } + findMutations($target, $oldstate); + return dirty; + } + + /** + * Utility + * Cones a element into a custom data structure designed for comparision. https://gist.github.com/megawac/8201012 + * + * @param {Node} $target + * @param {!Object} config : A custom mutation config + * @return {!Object} : Cloned data structure + */ + function clone($target, config) { + let recurse = true; // set true so childList we'll always check the first level + return (function copy($target) { + const elestruct: any = { + /** @type {Node} */ + node: $target, + }; + + // Store current character data of target text or comment node if the config requests + // those properties to be observed. + if ( + config.charData && + ($target.nodeType === 3 || $target.nodeType === 8) + ) { + elestruct.charData = $target.nodeValue; + } + // its either a element, comment, doc frag or document node + else { + // Add attr only if subtree is specified or top level and avoid if + // attributes is a document object (#13). + if (config.attr && recurse && $target.nodeType === 1) { + /** + * clone live attribute list to an object structure {name: val} + * @type {Object.} + */ + elestruct.attr = reduce( + $target.attributes, + function(memo, attr) { + if (!config.afilter || config.afilter[attr.name]) { + memo[attr.name] = getAttributeValue($target, attr); + } + return memo; + }, + {}, + ); + } + + // whether we should iterate the children of $target node + if ( + recurse && + (config.kids || + config.charData || + (config.attr && config.descendents)) + ) { + /** @type {Array.} : Array of custom clone */ + elestruct.kids = map($target.childNodes, copy); + } + + recurse = config.descendents; + } + return elestruct; + })($target); + } + + /** + * indexOf an element in a collection of custom nodes + * + * @param {NodeList} set + * @param {!Object} $node : A custom cloned node + * @param {number} idx : index to start the loop + * @return {number} + */ + function indexOfCustomNode(set, $node, idx) { + return indexOf(set, $node, idx, JSCompiler_renameProperty('node')); + } + + // using a non id (eg outerHTML or nodeValue) is extremely naive and will run into issues with nodes that may appear the same like
  • + let counter = 1; // don't use 0 as id (falsy) + /** @const */ + const expando = 'mo_id'; + + /** + * Attempt to uniquely id an element for hashing. We could optimize this for legacy browsers but it hopefully wont be called enough to be a concern + * + * @param {Node} $ele + * @return {(string|number)} + */ + function getElementId($ele) { + try { + return $ele.id || ($ele[expando] = $ele[expando] || counter++); + } catch (o_O) { + // ie <8 will throw if you set an unknown property on a text node + try { + return $ele.nodeValue; // naive + } catch (shitie) { + // when text node is removed: https://gist.github.com/megawac/8355978 :( + return counter++; + } + } + } + + /** + * **map** Apply a mapping function to each item of a set + * @param {Array|NodeList} set + * @param {Function} iterator + */ + function map(set, iterator) { + const results = []; + for (let index = 0; index < set.length; index++) { + results[index] = iterator(set[index], index, set); + } + return results; + } + + /** + * **Reduce** builds up a single result from a list of values + * @param {Array|NodeList|NamedNodeMap} set + * @param {Function} iterator + * @param {*} [memo] Initial value of the memo. + */ + function reduce(set, iterator, memo) { + for (let index = 0; index < set.length; index++) { + memo = iterator(memo, set[index], index, set); + } + return memo; + } + + /** + * **indexOf** find index of item in collection. + * @param {Array|NodeList} set + * @param {Object} item + * @param {number} idx + * @param {string} [prop] Property on set item to compare to item + */ + function indexOf(set, item, idx, prop) { + for (; /* idx = ~~idx*/ idx < set.length; idx++) { + // start idx is always given as this is internal + if ((prop ? set[idx][prop] : set[idx]) === item) return idx; + } + return -1; + } + + /** + * @param {Object} obj + * @param {(string|number)} prop + * @return {boolean} + */ + function has(obj, prop) { + return obj[prop] !== undefined; // will be nicely inlined by gcc + } + + // GCC hack see https://stackoverflow.com/a/23202438/1517919 + function JSCompiler_renameProperty(a) { + return a; + } + + return MutationObserver; + })(void 0); From 612d0f2be3183fabc6eeed2091d1857cf442484a Mon Sep 17 00:00:00 2001 From: Andrew Musgrave Date: Tue, 4 Jun 2019 17:23:27 -0400 Subject: [PATCH 2/2] changelog --- UNRELEASED.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/UNRELEASED.md b/UNRELEASED.md index ff05efd7f83..e32453f79ae 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -40,6 +40,8 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f ### Code quality +- Updated `PositionedOverlay` to no longer use `componentWillReceiveProps`([#1621](https://github.com/Shopify/polaris-react/pull/1621)) + ### Deprecations - `Card` `secondaryFooterAction` is now deprecated. Set an array of secondary actions on the `secondaryFooterActions` prop instead [#1625](https://github.com/Shopify/polaris-react/pull/1625)