Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update svelte to 3.37.0 #152

Merged
merged 23 commits into from
Jun 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 30 additions & 11 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
version: 2.1

supported-svelte-versions: &supported-svelte-versions ["local", "3.29.4"]

workflows:
version: 2
build:
build_and_test:
jobs:
- build_and_test
- build_and_test:
matrix:
parameters:
svelte-version: *supported-svelte-versions
jobs:
build_and_test:
parameters:
svelte-version:
type: string
description: Overrides the Svelte version. `local` refers to the locally-installed version.
default: "local"
docker:
- image: circleci/node:14-buster-browsers
steps:
Expand All @@ -25,9 +34,27 @@ jobs:
- run:
name: Yarn install
command: yarn install --immutable
- save_cache:
name: Save yarn cache
key: yarn-v1-{{ checksum "yarn.lock" }}
paths:
- ~/.cache/yarn
- run:
name: Lint
command: yarn lint
- run:
name: Bundlesize tests
command: yarn benchmark:bundlesize
- when:
condition:
not:
equal: [ <<parameters.svelte-version>>, "local" ]
steps:
- run:
name: Override version of svelte@<<parameters.svelte-version>>
# Do --ignore-scripts because we don't actually want the old version of Svelte to compile
# the picker; just get injected at runtime. This is how `emoji-picker-element/svelte` is used.
command: yarn add svelte@<<parameters.svelte-version>> --dev --ignore-scripts
- run:
name: Unit tests
command: yarn test
Expand All @@ -37,13 +64,5 @@ jobs:
- run:
name: Leak tests
command: yarn test:leak
- run:
name: Bundlesize tests
command: yarn benchmark:bundlesize
- save_cache:
name: Save yarn cache
key: yarn-v1-{{ checksum "yarn.lock" }}
paths:
- ~/.cache/yarn
- store_artifacts:
path: coverage
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,8 @@ import Picker from 'emoji-picker-element/svelte';

`svelte.js` is the same as `picker.js`, except it `import`s Svelte rather than bundling it.

While this option can reduce your bundle size, note that it only works if your Svelte version is compatible with `emoji-picker-element`'s Svelte version. You can check [the tests](https://github.com/nolanlawson/emoji-picker-element/blob/master/.circleci/config.yml) to see which Svelte versions are tested.

## Data and offline

### Data source and JSON format
Expand Down
8 changes: 6 additions & 2 deletions config/jest.setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import FDBFactory from 'fake-indexeddb/build/FDBFactory'
import FDBKeyRange from 'fake-indexeddb/build/FDBKeyRange'
import { Crypto } from '@peculiar/webcrypto'
import { ResizeObserver } from 'd2l-resize-aware/resize-observer-module.js'
import { deleteDatabase } from '../src/database/databaseLifecycle'

if (!global.performance) {
global.performance = {}
Expand All @@ -25,13 +26,16 @@ global.ResizeObserver = ResizeObserver
process.env.NODE_ENV = 'test'

global.IDBKeyRange = FDBKeyRange
global.indexedDB = new FDBFactory()

beforeAll(() => {
jest.spyOn(global.console, 'log').mockImplementation()
jest.spyOn(global.console, 'warn').mockImplementation()
jest.spyOn(global.console, 'error').mockImplementation()
})

beforeEach(() => {
global.indexedDB = new FDBFactory() // fresh indexedDB for every test
afterEach(async () => {
// fresh indexedDB for every test
const dbs = await global.indexedDB.databases()
await Promise.all(dbs.map(({ name }) => deleteDatabase(name)))
})
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"benchmark:run-bundlesize": "bundlesize",
"benchmark:storage": "cross-env PERF=1 run-s build:rollup && run-p --race test:adhoc benchmark:storage:test",
"benchmark:storage:test": "node ./test/storage/test.js",
"test:leak": "run-s build:rollup && run-p --race test:leak:server test:leak:test",
"test:leak": "run-p --race test:leak:server test:leak:test",
"test:leak:server": "node ./test/leak/server.js",
"test:leak:test": "node ./test/leak/test.js",
"dev": "run-p --race dev:rollup dev:server",
Expand Down Expand Up @@ -120,7 +120,7 @@
"stylelint": "^13.13.1",
"stylelint-config-recommended-scss": "^4.2.0",
"stylelint-scss": "^3.19.0",
"svelte": "3.29.4",
"svelte": "3.37.0",
"svelte-jester": "nolanlawson/svelte-jester#auto-preprocess",
"svelte-preprocess": "^4.7.3",
"svgo": "^2.3.0",
Expand Down Expand Up @@ -189,7 +189,7 @@
"bundlesize": [
{
"path": "./bundle.js",
"maxSize": "41.2 kB",
"maxSize": "41.4 kB",
"compression": "none"
},
{
Expand Down
26 changes: 19 additions & 7 deletions src/picker/PickerElement.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,29 @@
import SveltePicker from './components/Picker/Picker.svelte'
import { DEFAULT_DATA_SOURCE, DEFAULT_LOCALE } from '../database/constants'
import { runAll } from './utils/runAll'

export default class Picker extends SveltePicker {
export default class PickerElement extends SveltePicker {
constructor (props) {
performance.mark('initialLoad')
// Make the API simpler, directly pass in the props
super({ props })
super({
props: {
// Set defaults
locale: DEFAULT_LOCALE,
dataSource: DEFAULT_DATA_SOURCE,
...props
}
})
}

disconnectedCallback () {
// Have to explicitly destroy the component to avoid memory leaks.
// See https://github.com/sveltejs/svelte/issues/1152
console.log('disconnectedCallback')
this.$destroy()
// For Svelte v <3.33.0, we have to run the destroy logic ourselves because it doesn't have this fix:
// https://github.com/sveltejs/svelte/commit/d4f98f
// We can safely just run on_disconnect and on_destroy to cover all versions of Svelte. In older versions
// the on_destroy array will have length 1, whereas in more recent versions it'll be on_disconnect instead.
// TODO: remove this when we drop support for Svelte < 3.33.0
runAll(this.$$.on_destroy)
runAll(this.$$.on_disconnect)
}

static get observedAttributes () {
Expand All @@ -28,4 +40,4 @@ export default class Picker extends SveltePicker {
}
}

customElements.define('emoji-picker', Picker)
customElements.define('emoji-picker', PickerElement)
73 changes: 37 additions & 36 deletions src/picker/components/Picker/Picker.html
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
<div class="indicator-wrapper">
<div class="indicator"
style={indicatorStyle}
use:calculateIndicatorWidth>
bind:this={indicatorElement}>
</div>
</div>

Expand All @@ -117,41 +117,42 @@
on:click={onEmojiClick}
bind:this={tabpanelElement}
>
{#each currentEmojisWithCategories as emojiWithCategory, i (emojiWithCategory.category)}
<div
id="menu-label-{i}"
class="category {currentEmojisWithCategories.length > 1 ? '' : 'gone'}"
aria-hidden="true">
<!-- This logic is a bit complicated in order to avoid a flash of the word "Custom" while switching
from a tabpanel with custom emoji to a regular group. I.e. we don't want it to suddenly flash
from "Custom" to "Smileys and emoticons" when you click the second nav button. -->
{searchMode ? i18n.searchResultsLabel : (
emojiWithCategory.category ? emojiWithCategory.category : (
currentEmojisWithCategories.length > 1 ? i18n.categories.custom : i18n.categories[currentGroup.name]
)
)}
</div>
<div class="emoji-menu"
role={searchMode ? 'listbox' : 'menu'}
aria-labelledby="menu-label-{i}"
id={searchMode ? 'search-results' : ''}
use:calculateEmojiGridWidth>
{#each emojiWithCategory.emojis as emoji, i (emoji.id)}
<button role={searchMode ? 'option' : 'menuitem'}
aria-selected={searchMode ? i == activeSearchItem : ''}
aria-label={labelWithSkin(emoji, currentSkinTone)}
title={emoji.title}
class="emoji {searchMode && i === activeSearchItem ? 'active' : ''}"
id="emo-{emoji.id}">
{#if emoji.unicode}
{unicodeWithSkin(emoji, currentSkinTone)}
{:else}
<img class="custom-emoji" src={emoji.url} alt="" loading="lazy" />
{/if}
</button>
{/each}
</div>
{/each}
<div bind:this={tabpanelInnerElement}>
{#each currentEmojisWithCategories as emojiWithCategory, i (emojiWithCategory.category)}
<div
id="menu-label-{i}"
class="category {currentEmojisWithCategories.length > 1 ? '' : 'gone'}"
aria-hidden="true">
<!-- This logic is a bit complicated in order to avoid a flash of the word "Custom" while switching
from a tabpanel with custom emoji to a regular group. I.e. we don't want it to suddenly flash
from "Custom" to "Smileys and emoticons" when you click the second nav button. -->
{searchMode ? i18n.searchResultsLabel : (
emojiWithCategory.category ? emojiWithCategory.category : (
currentEmojisWithCategories.length > 1 ? i18n.categories.custom : i18n.categories[currentGroup.name]
)
)}
</div>
<div class="emoji-menu"
role={searchMode ? 'listbox' : 'menu'}
aria-labelledby="menu-label-{i}"
id={searchMode ? 'search-results' : ''}>
{#each emojiWithCategory.emojis as emoji, i (emoji.id)}
<button role={searchMode ? 'option' : 'menuitem'}
aria-selected={searchMode ? i == activeSearchItem : ''}
aria-label={labelWithSkin(emoji, currentSkinTone)}
title={emoji.title}
class="emoji {searchMode && i === activeSearchItem ? 'active' : ''}"
id="emo-{emoji.id}">
{#if emoji.unicode}
{unicodeWithSkin(emoji, currentSkinTone)}
{:else}
<img class="custom-emoji" src={emoji.url} alt="" loading="lazy" />
{/if}
</button>
{/each}
</div>
{/each}
</div>
</div>
<div class="favorites emoji-menu {message ? 'gone': ''}"
role="menu"
Expand Down
55 changes: 32 additions & 23 deletions src/picker/components/Picker/Picker.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import Database from '../../ImportedDatabase'
import enI18n from '../../i18n/en'
import { groups as defaultGroups, customGroup } from '../../groups'
import { DEFAULT_LOCALE, DEFAULT_DATA_SOURCE } from '../../../database/constants'
import { MIN_SEARCH_TEXT_LENGTH, NUM_SKIN_TONES } from '../../../shared/constants'
import { requestIdleCallback } from '../../utils/requestIdleCallback'
import { hasZwj } from '../../utils/hasZwj'
Expand All @@ -22,9 +21,10 @@ import { summarizeEmojisForUI } from '../../utils/summarizeEmojisForUI'
import * as widthCalculator from '../../utils/widthCalculator'
import { checkZwjSupport } from '../../utils/checkZwjSupport'
import { requestPostAnimationFrame } from '../../utils/requestPostAnimationFrame'
import { onMount, onDestroy, tick } from 'svelte'
import { onMount, tick } from 'svelte'
import { requestAnimationFrame } from '../../utils/requestAnimationFrame'
import { uniq } from '../../../shared/uniq'
import { runAll } from '../../utils/runAll'

// public
let locale = null
Expand All @@ -44,6 +44,8 @@ let searchText = ''
let rootElement
let baselineEmoji
let tabpanelElement
let tabpanelInnerElement
let indicatorElement
let searchMode = false // eslint-disable-line no-unused-vars
let activeSearchItem = -1
let message // eslint-disable-line no-unused-vars
Expand Down Expand Up @@ -141,35 +143,42 @@ $: {
}
}

// TODO: this is a bizarre way to set these default properties, but currently Svelte
// renders custom elements in an odd way - props are not set when calling the constructor,
// but are only set later. This would cause a double render or a double-fetch of
// the dataSource, which is bad. Delaying with a microtask avoids this.
// See https://github.com/sveltejs/svelte/pull/4527
onMount(async () => {
await tick()
console.log('props ready: setting locale and dataSource to default')
locale = locale || DEFAULT_LOCALE
dataSource = dataSource || DEFAULT_DATA_SOURCE
onMount(() => {
const destroys = [
calculateIndicatorWidth(indicatorElement),
// The reason for the tabpanelInnerElement is that, if we measure the width on the tabpanelElement,
// then we don't always exclude the scrollbar. In Chrome/WebKit it does, in Firefox it does not.
calculateEmojiGridWidth(tabpanelInnerElement)
]

return async () => {
// TODO: using a workaround for Svelte actions never calling destroy() when used in
// custom elements. Instead of waiting for a destroy event, we use the mount/unmount
// lifecycle to clean up.
// https://github.com/sveltejs/svelte/issues/5989#issuecomment-796366910
runAll(destroys)
// Close the database when the component is disconnected. It will automatically reconnect anyway
// if the component is ever reconnected.
if (database) {
console.log('closing database')
try {
await database.close()
} catch (err) {
console.error(err) // only happens if the database failed to load in the first place, so we don't care
}
}
}
})

$: {
// API props like locale and dataSource are not actually set until the onMount phase
// https://github.com/sveltejs/svelte/pull/4522
if (locale && dataSource && (!database || (database.locale !== locale && database.dataSource !== dataSource))) {
console.log('creating database', { locale, dataSource })
database = new Database({ dataSource, locale })
}
}

onDestroy(async () => {
if (database) {
console.log('closing database')
try {
await database.close()
} catch (err) {
console.error(err) // only happens if the database failed to load in the first place, so we don't care
}
}
})

//
// Global styles for the entire picker
//
Expand Down
1 change: 1 addition & 0 deletions src/picker/utils/runAll.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const runAll = funcs => (funcs && funcs.forEach(func => func()))
9 changes: 4 additions & 5 deletions src/picker/utils/widthCalculator.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,10 @@ export function calculateWidth (node, onUpdate) {
))
}

return {
destroy () {
if (resizeObserver) {
resizeObserver.disconnect()
}
// cleanup function (called on disconnect)
return () => {
if (resizeObserver) {
resizeObserver.disconnect()
}
}
}
Loading