-
-
Notifications
You must be signed in to change notification settings - Fork 531
fix: import emoji-mart dynamically like (#950) #1491
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,13 +9,32 @@ import { | |
} from "../../schema/index.js"; | ||
import { DefaultGridSuggestionItem } from "./DefaultGridSuggestionItem.js"; | ||
|
||
let data: | ||
| Promise<{ | ||
default: EmojiMartData; | ||
}> | ||
| undefined; | ||
|
||
// Temporary fix for https://github.com/missive/emoji-mart/pull/929 | ||
let emojiMart: typeof import("emoji-mart") | undefined; | ||
let emojiData: EmojiMartData | undefined; | ||
|
||
async function loadEmojiMart() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Decided to make both of these the same, and also made them Node.js safe |
||
if (!emojiMart || !emojiData) { | ||
// load dynamically because emoji-mart doesn't specify type: module and breaks in nodejs | ||
const [emojiMartModule, emojiDataModule] = await Promise.all([ | ||
import("emoji-mart"), | ||
// use a dynamic import to encourage bundle-splitting | ||
// and a smaller initial client bundle size | ||
import("@emoji-mart/data"), | ||
]); | ||
|
||
emojiMart = | ||
"default" in emojiMartModule ? emojiMartModule.default : emojiMartModule; | ||
emojiData = | ||
"default" in emojiDataModule | ||
? (emojiDataModule.default as EmojiMartData) | ||
: (emojiDataModule as EmojiMartData); | ||
|
||
emojiMart.init({ data: emojiData }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this needs an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, wasn't clear that it was async in the docs https://github.com/missive/emoji-mart?tab=readme-ov-file#-emoji-component , but won't hurt to await |
||
} | ||
|
||
return { emojiMart, emojiData }; | ||
} | ||
|
||
export async function getDefaultEmojiPickerItems< | ||
BSchema extends BlockSchema, | ||
|
@@ -29,23 +48,11 @@ export async function getDefaultEmojiPickerItems< | |
return []; | ||
} | ||
|
||
if (!data) { | ||
// use a dynamic import to encourage bundle-splitting | ||
// and a smaller initial client bundle size | ||
|
||
data = import("@emoji-mart/data") as any; | ||
|
||
// load dynamically because emoji-mart doesn't specify type: module and breaks in nodejs | ||
emojiMart = await import("emoji-mart"); | ||
const emojiMartData = (await data)!.default; | ||
await emojiMart.init({ data: emojiMartData }); | ||
} | ||
|
||
const emojiMartData = (await data)!.default; | ||
const { emojiData } = await loadEmojiMart(); | ||
|
||
const emojisToShow = | ||
query.trim() === "" | ||
? Object.values(emojiMartData.emojis) | ||
? Object.values(emojiData.emojis) | ||
: ((await emojiMart!.SearchIndex.search(query)) as Emoji[]); | ||
|
||
return emojisToShow.map((emoji) => ({ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fyi, the reason data was global and could be a promise would be to prevent duplicate loads when
loadEmojiMart
is called twice in a row.Didn't help readability and it might be a premature optimization so I'm ok with the new version as long as we don't see this causing any issues
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I can de-dupe easily. I'll pull that in.