From 586ba4375bda30b2f7aca195e50d88d5fc244654 Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Fri, 16 Aug 2024 08:44:45 -0700 Subject: [PATCH] perf: avoid unnecessary re-renders (#437) --- src/picker/components/Picker/Picker.js | 11 ++++-- src/picker/components/Picker/reactivity.js | 22 ++++++----- src/picker/utils/checkZwjSupport.js | 29 +++++++++++--- test/spec/picker/zwjEmoji.test.js | 44 ++++++++++++++++++++++ 4 files changed, 87 insertions(+), 19 deletions(-) create mode 100644 test/spec/picker/zwjEmoji.test.js diff --git a/src/picker/components/Picker/Picker.js b/src/picker/components/Picker/Picker.js index 60488d6f..fd2823a4 100644 --- a/src/picker/components/Picker/Picker.js +++ b/src/picker/components/Picker/Picker.js @@ -427,10 +427,13 @@ export function createRoot (shadowRoot, props) { }) function checkZwjSupportAndUpdate (zwjEmojisToCheck) { - checkZwjSupport(zwjEmojisToCheck, refs.baselineEmoji, emojiToDomNode) - // force update - // eslint-disable-next-line no-self-assign - state.currentEmojis = state.currentEmojis + const allSupported = checkZwjSupport(zwjEmojisToCheck, refs.baselineEmoji, emojiToDomNode) + if (!allSupported) { + console.log('Not all ZWJ emoji are supported, forcing re-render') + // Force update. We only do this if there are any unsupported ZWJ characters since otherwise, + // for browsers that support all emoji, it would be an unnecessary extra re-render. + state.currentEmojis = [...state.currentEmojis] + } } function isZwjSupported (emoji) { diff --git a/src/picker/components/Picker/reactivity.js b/src/picker/components/Picker/reactivity.js index a9b9fe8f..fd3c96f8 100644 --- a/src/picker/components/Picker/reactivity.js +++ b/src/picker/components/Picker/reactivity.js @@ -49,16 +49,18 @@ export function createState (abortSignal) { return target[prop] }, set (target, prop, newValue) { - target[prop] = newValue - const observers = propsToObservers.get(prop) - if (observers) { - for (const observer of observers) { - dirtyObservers.add(observer) - } - if (!queued) { - recursionDepth = 0 - queued = true - queueMicrotask(flush) + if (target[prop] !== newValue) { + target[prop] = newValue + const observers = propsToObservers.get(prop) + if (observers) { + for (const observer of observers) { + dirtyObservers.add(observer) + } + if (!queued) { + recursionDepth = 0 + queued = true + queueMicrotask(flush) + } } } return true diff --git a/src/picker/utils/checkZwjSupport.js b/src/picker/utils/checkZwjSupport.js index 66919c31..7043a72d 100644 --- a/src/picker/utils/checkZwjSupport.js +++ b/src/picker/utils/checkZwjSupport.js @@ -3,8 +3,22 @@ import { supportedZwjEmojis } from './emojiSupport' let baselineEmojiWidth +// only used in tests +let simulateBrowserNotSupportingZWJEmoji = false +export function setSimulateBrowserNotSupportingZWJEmoji (value) { + simulateBrowserNotSupportingZWJEmoji = value +} + +/** + * Check if the given emojis containing ZWJ characters are supported by the current browser (don't render + * as double characters) and return true if all are supported. + * @param zwjEmojisToCheck + * @param baselineEmoji + * @param emojiToDomNode + */ export function checkZwjSupport (zwjEmojisToCheck, baselineEmoji, emojiToDomNode) { performance.mark('checkZwjSupport') + let allSupported = true for (const emoji of zwjEmojisToCheck) { const domNode = emojiToDomNode(emoji) const emojiWidth = calculateTextWidth(domNode) @@ -15,14 +29,19 @@ export function checkZwjSupport (zwjEmojisToCheck, baselineEmoji, emojiToDomNode // against are the ones that are 2x the size, because those are truly broken (person with red hair = person with // floating red wig, black cat = cat with black square, polar bear = bear with snowflake, etc.) // So here we set the threshold at 1.8 times the size of the baseline emoji. - const supported = emojiWidth / 1.8 < baselineEmojiWidth + const supported = !simulateBrowserNotSupportingZWJEmoji && emojiWidth / 1.8 < baselineEmojiWidth supportedZwjEmojis.set(emoji.unicode, supported) - /* istanbul ignore next */ + if (!supported) { - console.log('Filtered unsupported emoji', emoji.unicode, emojiWidth, baselineEmojiWidth) - } else if (emojiWidth !== baselineEmojiWidth) { - console.log('Allowed borderline emoji', emoji.unicode, emojiWidth, baselineEmojiWidth) + allSupported = false + console.log('Filtered unsupported ZWJ emoji', emoji.unicode, emojiWidth, baselineEmojiWidth) + } + + /* istanbul ignore next */ + if (supported && emojiWidth !== baselineEmojiWidth) { + console.log('Allowed borderline ZWJ emoji', emoji.unicode, emojiWidth, baselineEmojiWidth) } } performance.measure('checkZwjSupport', 'checkZwjSupport') + return allSupported } diff --git a/test/spec/picker/zwjEmoji.test.js b/test/spec/picker/zwjEmoji.test.js new file mode 100644 index 00000000..f46c3f45 --- /dev/null +++ b/test/spec/picker/zwjEmoji.test.js @@ -0,0 +1,44 @@ +import Picker from '../../../src/picker/PickerElement' +import { ALL_EMOJI, basicAfterEach, basicBeforeEach, truncatedEmoji } from '../shared' +import { setSimulateBrowserNotSupportingZWJEmoji } from '../../../src/picker/utils/checkZwjSupport.js' +import { supportedZwjEmojis } from '../../../src/picker/utils/emojiSupport.js' +import { getAllByRole, getByRole, queryAllByRole, waitFor } from '@testing-library/dom' +import { groups } from '../../../src/picker/groups.js' + +describe('ZWJ emoji', () => { + const simulations = [false, true] + + simulations.forEach(simulateBrowserNotSupportingZWJ => { + describe(`simulateBrowserNotSupportingZWJ=${simulateBrowserNotSupportingZWJ}`, () => { + beforeEach(async () => { + await basicBeforeEach() + setSimulateBrowserNotSupportingZWJEmoji(simulateBrowserNotSupportingZWJ) + }) + afterEach(async () => { + await basicAfterEach() + setSimulateBrowserNotSupportingZWJEmoji(false) + supportedZwjEmojis.clear() // reset global cache after each test + }) + + test('shows or hides ZWJ emoji appropriately', async () => { + console.log(truncatedEmoji) + const picker = new Picker({ + dataSource: ALL_EMOJI + }) + + document.body.appendChild(picker) + + const container = picker.shadowRoot.querySelector('.picker') + + await waitFor(() => expect(getAllByRole(container, 'tab')).toHaveLength(groups.length)) + + getByRole(container, 'tab', { name: 'Flags' }).click() + + // checkered flag is shown because it's not ZWJ, should always be shown + await waitFor(() => expect(getByRole(container, 'menuitem', { name: /🏁/ })).toBeVisible()) + // pirate flag is a ZWJ combining black flag plus skull and crossbones, should be hidden if no browser support + await waitFor(() => expect(queryAllByRole(container, 'menuitem', { name: /🏴‍☠️/ })).toHaveLength(simulateBrowserNotSupportingZWJ ? 0 : 1)) + }) + }) + }) +})