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

Add a .extend(...) option to expand upon the built-in dictionary #144

Open
JoshuaKGoldberg opened this issue Nov 17, 2023 · 2 comments
Open
Labels
status: accepting prs Please, send a pull request to resolve this! type: feature New enhancement or request

Comments

@JoshuaKGoldberg
Copy link
Collaborator

JoshuaKGoldberg commented Nov 17, 2023

Forking @bigpay-ali's #132 (comment):

Hello beautiful humans, I love what you did with this lib and thank you for providing this library for developer like me to build on top of it, is there a way we can get this fix merged? I desprately need some emojis which are missing from your lib. Maybe we can add a .extend(":emoji-code","✅") function, just in case some emojis are missing, developer can update it and make it searchable across project?

I like this idea. Even after the latest emojilib gets merged in, it's possible folks will want to add their own context/project-specific emoji extensions.

One difficulty with this is that right now, all node-emoji APIs are exported standalone: e.g. import { findByName } from "node-emoji". This new extension API would need to either:

  • Modify the global-ish data used by the entire node-emoji module
  • Create and return a new object with APIs

I'd lean towards the latter personally, as modifying global state is a worrisome thing to me.

My starting API proposal:

import { extend } from "node-emoji";

const emojiBank = extend({
  emojis: [
    { code: "yeehaw", emoji: "🤠" }
  ]
});

This differs from the original comment by being more explicit and open to later expansion, at the cost of extra verbosity.

@JoshuaKGoldberg JoshuaKGoldberg added type: feature New enhancement or request status: in discussion Not yet ready for implementation or a pull request labels Nov 17, 2023
@omnidan
Copy link
Owner

omnidan commented Nov 18, 2023

@JoshuaKGoldberg I really like the idea of making the library more extensible and it would likely solve a lot of issues people are having with the library not supporting certain codes for certain emoji. I also agree that returning a new object with the same APIs as node-emoji is the way to go. That way you could even have multiple instances of node-emoji that all resolve them differently. It is way more flexible and less prone to tricky bugs that can arise from modifying the global state of the library.

@JoshuaKGoldberg
Copy link
Collaborator Author

One month and no additional inputs - marking as accepting PRs! 🚀

Note that this probably isn't a trivial change. It'll require making all sort of stuff in node-emoji per-instance.

@JoshuaKGoldberg JoshuaKGoldberg added status: accepting prs Please, send a pull request to resolve this! and removed status: in discussion Not yet ready for implementation or a pull request labels Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepting prs Please, send a pull request to resolve this! type: feature New enhancement or request
Projects
None yet
Development

No branches or pull requests

2 participants