Skip to content

Commit

Permalink
perf: avoid unnecessary re-renders (#437)
Browse files Browse the repository at this point in the history
  • Loading branch information
nolanlawson authored Aug 16, 2024
1 parent a14ffb7 commit 586ba43
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 19 deletions.
11 changes: 7 additions & 4 deletions src/picker/components/Picker/Picker.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
22 changes: 12 additions & 10 deletions src/picker/components/Picker/reactivity.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 24 additions & 5 deletions src/picker/utils/checkZwjSupport.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}
44 changes: 44 additions & 0 deletions test/spec/picker/zwjEmoji.test.js
Original file line number Diff line number Diff line change
@@ -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))
})
})
})
})

0 comments on commit 586ba43

Please sign in to comment.