node-emoji icon indicating copy to clipboard operation
node-emoji copied to clipboard

v2 Refactor, Refactored

Open JoshuaKGoldberg opened this issue 3 years ago • 4 comments

#86 but with the changes suggested by maintainers and/or proposed by me as comments. Most notably:

  • Revamped README.md, including linking to new dependencies
  • Removes standard and much of the excess .git* metadata
  • Uses Jest as the test runner instead of Ava
  • Switches replace from removeSpaces = true to preserveSpaces = false

These are the breaking changes I can spot compared to v1 (please comment if there are others!):

  • Uses emojilib instead of in-house emojis, causing slight differences in emoji names
  • emojify takes in fallback and/or format as members of an object parameter instead of in series
  • hasEmoji is now called has to match with other single-word API names
  • replace takes in preserveSpaces as an optional member of an optional object parameter instead of as its own parameter

Intentionally leaves the following major changes out to be discussed separately:

  • ESM modules
  • Prettier (though I did add a .prettierrc.json to help editor Prettier extensions match the existing settings)
  • TypeScript

Closes #55, Closes #57, Closes #62, Closes #66, Closes #77.

Intentionally based on #86's v2 branch to preserve @Richienb as an author in history.

JoshuaKGoldberg avatar Nov 18 '21 22:11 JoshuaKGoldberg

Removes standard and much of the excess .git* metadata

By removing standard, we are left with no linter at all. I'd just add xo which can be used as a formatter and linter.

Uses Jest as the test runner instead of Ava

What's the motivation behind this change?

ESM modules

Yes, this is definately the path we want to be going down. Perhaps it could be ported to TypeScript as well.

Richienb avatar Nov 19 '21 03:11 Richienb

👋 @Richienb! I'm just going off of the recommendations in #86. This is my first node-emoji PR so I don't want to put forward any opinions of my own too strongly 😄

  • For standard / xo: https://github.com/omnidan/node-emoji/pull/86/files#r752410060 (hoping to push into a followup issue+PR to keep the initial v2 one smaller)
  • For Jest: https://github.com/omnidan/node-emoji/pull/86/files#r752418062 (cc @omnidan; Jest is also what I'm used to)

JoshuaKGoldberg avatar Nov 19 '21 07:11 JoshuaKGoldberg

Ping @omnidan, are you still interested in this work being completed?

JoshuaKGoldberg avatar Apr 03 '22 23:04 JoshuaKGoldberg

@JoshuaKGoldberg yes that would be great! I'm always on our discord so feel free to ping me there for discussions 😄

omnidan avatar Apr 16 '22 17:04 omnidan

Don't know if we've discussed this before in the repo, but we could use Intl.Segmenter which is marginally better than char-regex but requires Node.js >16.

Richienb avatar Mar 26 '23 12:03 Richienb

But otherwise, this is pretty nice

Richienb avatar Mar 26 '23 12:03 Richienb