missive / emoji-mart

🏪 One component to pick them all
https://missiveapp.com/open/emoji-mart
MIT License
8.67k stars 835 forks source link

Split out emoji library from emoji picker. 👻 #86

Closed samkelleher closed 6 years ago

samkelleher commented 7 years ago

Just want to get a comment form the emoji-mart contributors.

I'm going to work on a fork that will split out using Higher Order Components the rendering of the emoji-picker and the datasource.

At present, they are all bundled together, the picker imports directly the generate data.js file. However, I believe this complicates the the release cycle as often the underlying dataset will change more than the picker does.

Secondly, in my use case, the application uses the emoji dataset elsewhere. For example, in an rich text editor autocomplete. And in string replace to replace emoji with images when viewed on a older platform, or Windows. So it doesn't make sense to have two copies of the emoji library.

Since making the changes required would likely result in an API change I wanted to get a feel for comments first before I started, especially related to component naming.

Something like:

Works as normal, uses localstorage and internal data library.

<Picker />

Picker would just wrap the SimplePicker and provide it with the internal data and localstorage functions. However, my application could consume the lower level component directly, which I've called SimplePicker which would expect the storage/dataset functions to be injected into it.

const data = application.loadEmojiDataLib();
const saveSkinTone = skinTone => application.storeUserPreference({ skinTone });

<SimplePicker data={data} onSkinTonePreferenceChange={saveSkinTone} />

This means that by referencing only the SimplePicker, webpack would not bundle the included emoji dataset in my application code.

Thoughts / comments? 💬

savardc commented 7 years ago

We also use the dataset separately from the picker and having an easier way to query it would be a big improvement

EtienneLem commented 7 years ago

That would work for me 😄

slvnperron commented 7 years ago

@samkelleher Have you done that? I also need to split data from the bundle

samkelleher commented 7 years ago

@slvnperron I did actually and finished the other day, however, by the time I finished and moved things about I have something thats radically different to emoii-mart. Visually similar, but by time I had moved stuff around and generally optimised and tweaked it I essentially have something new. Need to figure out where to take it from here.

mismith commented 7 years ago

What's the status of this?

samkelleher commented 7 years ago

@mismith I need to figure out what to do since when I split out the emoji dataset to essentially recreate an emoji picker using dumb components decoubled from their dataset, it obviously is a breaking change to emoji-mart so there is no clear upgrade path or replacement. I could continue to refactor and try preserve the original interface but this would take a lot of work and I've already completed the part I needed.

mismith commented 7 years ago

Gotcha, thanks for the clarification.

nolanlawson commented 7 years ago

This should be possible now?

import data from 'emoji-mart/dist/data/data.js'
import Picker from 'emoji-mart/dist/components/picker.js'
EtienneLem commented 6 years ago

This should now be possible since v2.6.0, see section in README https://github.com/missive/emoji-mart#datasets

I also noticed some places where the provided JSON for each set (in emoji-mart/data) can be optimized to reduce the size a little bit more. I’ll be updating that soon. Let me know if you have any questions regarding providing a custom data file.

dmfrancisco commented 6 years ago

@EtienneLem That's great, thank you 👍 I think there may be something wrong with the implementation or instructions (or maybe I'm misunderstanding) though.

I created an app and added emoji-mart v2.6.1:

import 'emoji-mart/css/emoji-mart.css';
import { Picker } from 'emoji-mart';

class App extends Component {
  render() {
    return (
      <Picker set='emojione' />
    );
  }
}

The js file after building weights 118.52 KB gzipped. I then followed the instructions in the readme:

import 'emoji-mart/css/emoji-mart.css';
import data from 'emoji-mart/data/emojione.json';
import { NimblePicker } from 'emoji-mart';

class App extends Component {
  render() {
    return (
      <NimblePicker set='emojione' data={data} />
    );
  }
}

After building it now weights 183.15 KB (+64.63 KB) gzipped. Using source-map-explorer you can see that the latter (page on the right) includes both all.json and emojione.json:

emoji-mart

EtienneLem commented 6 years ago

@dmfrancisco I see. Can you try import { NimblePicker } from 'emoji-mart/dist-es'? I believe to take advantage of tree-shaking it requires ES modules (which dist-es should have).

Let me know if that works, will update the docs accordingly if it does.

dmfrancisco commented 6 years ago

I'm afraid I got the same result 😕 The demo I'm using is here and I'm testing with:

yarn build
npm install -g source-map-explorer
source-map-explorer build/static/js/main.<hash>.js
EtienneLem commented 6 years ago

Thanks for the demo! I believe it could be an issue with Webpack config when using create-react-app, I did a demo from scratch (w/ Webpack 4) and it feels like it’s working(?):

Demo: https://github.com/EtienneLem/emoji-mart-tree-shaking source-map-explorer:

screen shot 2018-05-24 at 12 42 22 pm
dmfrancisco commented 6 years ago

Interesting 👍 I quickly tried to clone and build your demo and the generated main.js weighted 1.01 MiB according to webpack, while when I replaced the NimblePicker with Picker and removed the data import it reduced to 618 KiB.

But I may have done something wrong! I'll take a closer look as soon as possible.

bramchi commented 6 years ago

I'm also seeing all.json show up together with twitter.json when I use the NimblePicker and set it to use the twitter.json data.

Using emoji-mart 2.6.1 with webpack 3.11.0, all the following ways of importing result in all.json ánd twitter.json in the bundle:

import { NimblePicker } from 'emoji-mart' import { NimblePicker } from 'emoji-mart/dist-es' import NimblePicker from 'emoji-mart/dist/components/picker/nimble-picker' import NimblePicker from 'emoji-mart/dist-es/components/picker/nimble-picker'

Any ideas/suggestions for a patch/solution are welcome 😄 Or is Webpack 3 not supported? Current situation doesn't allow me to upgrade to Webpack 4.

Kubster96 commented 6 years ago

@bramchi I have the same situation, using Webpack 4.12.1

piotrooo commented 6 years ago

Any news with this?

EtienneLem commented 6 years ago

I just realized that this file (https://github.com/missive/emoji-mart/blob/master/src/utils/data.js) uses module.exports instead of export. I’m pretty sure that’s because this file is also used by https://github.com/missive/emoji-mart/blob/master/scripts/build.js (which doesn’t go through Babel so it was throwing when requiring).

Although utils/data.js isn’t requiring any file, is the module.exports alone enough to nullify Webpack’s tree shaking?

bramchi commented 6 years ago

What I ended up doing as a workaround is cloning the package into my project and replacing all references to all.json in the component with the specific twitter.json dataset I want to use. Makes for a 'whopping' 50kB smaller build... haha oh well ¯\_(ツ)_/¯

Kubster96 commented 6 years ago

@bramchi I used NimblePicker and NimbleEmoji with messanger emojis (which included all.json and messanger.json) then i replaced all.json with empty json like this {}, so eventually it decreased build by 500 kB and still i could use in Nimble components any emoji set i liked.

EtienneLem commented 6 years ago

So I just published v2.7.0 which completely removes module.exports from the dist-es folder. Hopefully that’ll help w/ Webpack tree shaking.

peacepostman commented 6 years ago

Hello, we are still experiencing the same issue even when using NimblePicker and NimbleEmoji and specifying data import. Is anyone still in the same situation ?

Webpack version is 4.12.0 and Emoji-mart is 2.7.0

ajbeaven commented 6 years ago

Same issue here. 2.8.1 with NimblePicker loads both all.json and {set}.json. https://github.com/missive/emoji-mart/issues/229 has some suggestion on how to fix.

towfiqi commented 5 years ago

This is still an issue. The NimblePicker also includes the all.json file.