zzarcon / gh-emoji

Github emoji parsing done right :point_up: :pray: :point_up_2: :clap: :ok_hand:
http://zzarcon.github.io/gh-emoji/
MIT License
160 stars 7 forks source link

Admit custom emoji data #3

Open zzarcon opened 8 years ago

ryyppy commented 8 years ago

My proposal:

More general functions should state all required parameters in their definition:

// src/renderHTML.js
type ParseOptions = {
  classNames?: string,
  delimiterRegex?: RegExp,
};

type EmojiMap = {[id: string]: string};

export default function renderHTML(text: string, emojiMap: EmojiMap, options: ParseOptions = {}): string {
   // Create output according to input parameters, never alter any passed arguments
}

The module interface is responsible to hold the state (loaded emoji map etc.)

// src/index.js
import renderHTML from './renderHTML';
let emojiMap = null;

// Just use the renderHTML by augmenting the `emojiMap` in the passed parseOptions 
// (spread operator ensures that a new object is being created - Eventually we need a deep clone)
export const parse = (text: string, options: ParseOptions = {}) => renderHTML(text, {...options, emojiMap });
ryyppy commented 8 years ago

This also makes it very easy to test, since we now can just pass our fixture to the renderHTML function. For additional features in the future... for instance we want to add HTML-Sanitization, we can create higher-order functions to mix in behavior:

// src/sanitizeHTML.js
export default function sanitizeHTML(text: string): string { ... };
// Some application which requires HTML sanitization
import renderHTML from './renderHTML';
import sanitizeHTML from './sanitizeHTML';

const mySanitizedRenderHTML = (text: string, ...rest) => renderHTML(sanitizeHTML(text), ...rest);
rpunkfu commented 8 years ago

Should we also have some module like this?

// src/loadEmojis.js
import {endpoint} from './api';
let emojiMap = null;

export const loadEmojis = () => emojiMap ? emojiMap : fetch(endpoint)
  .then(response => response.json())
  .then(response => {
    emojiMap = response;
    resolve(emojiMap);
  })
ryyppy commented 8 years ago

I would make it more general, like fetchEmojis(endpoint) which requires the endpoint to return json in our specific format, and then create a export const fetchGithubEmojis() => fetchEmojis('https://github.com/emojis...');

rpunkfu commented 8 years ago

Wow, great! :) So something more like:

// src/fetchEmojis.js
let emojiMap = null;

export default const fetchEmojis = (endpoint) => emojiMap ? emojiMap : fetch(endpoint)
  .then(response => response.json())
  .then(response => {
    emojiMap = response;
    resolve(emojiMap);
  });
// src/fetchGithubEmojis.js
import {githubEndpoint} from './api/github';
import fetchEmojis from './fetchEmojis';

export default  const fetchGithubEmojis = fetchEmojis(githubEndpoint);
ryyppy commented 8 years ago

Exactly!

But your fetchEmojis should always return a Promise, to stay consistent ;-) Also I would not involve the emojiMap state in the lower level function... put this in the fetchGithubEmojis and you are good!

rpunkfu commented 8 years ago

Ohh, so it should look like that, right?

// src/fetchEmojis.js

export const fetchEmojis = endpoint => fetch(endpoint)
  .then(response => resolve(response.json()));
// src/fetchGithubEmojis.js

import { endpoint } from './api/github';
import { fetchEmojis } from './fetchEmojis';

let emojiMap = null;

export const fetchGithubEmojis = emojiMap ? emojiMap : fetchEmojis(endpoint)
  .then(response => {
    emojiMap = response;
    resolve(emojiMap);
  });
rpunkfu commented 8 years ago

Also, I think that methods like parse, add, etc will need a change as well. This is my example on how parse should work now:

// src/parseEmojis.js

const defaultOptions = {
  classNames: ''
  // etc... 
};

export default const parseEmojis = (text, emojiMap, options = defaultOptions) => {
  // Maybe line below is better than default param?
  // options = Object.assign({}, defaultOptions, options);
  resolve(parsedText);
};
// src/index.js

import { fetchGithubEmojis } from './fetchGithubEmojis';
import { parse } from './parseEmojis';

const text = 'Do you believe in :alien:...? :scream:';

fetchGithubEmojis()
  .then(emojiMap => parse(text, emojiMap))
  .then(parsedText => updatePreview(parsedText));

What do you think @zzarcon @ryyppy? :) I think parseEmojis can definitely be much more user friendly, since passing emojiMap makes it really ugly imo, but at the moment I don't have an idea how to fix it ;/ Love to see your ideas :)

ryyppy commented 8 years ago

So when we are changing the major API, I would think about a design like this:

//src/index.js

// Async operations
export function fetchEmojis(endpoint: string): Promise<EmojiMap> {};
export const fetchGithubEmojis = () => fetchEmojis('https://emojis.github.com');

// Parse operations
type ParseFn = (emojiMap: EmojiMap,options: ParserOptions = {}, text: string) => string;
export function parse(emojiMap: EmojiMap,options: ParserOptions = {}, text: string): string {};

export function createEmojiParse(emojiMap: EmojiMap, options: ParserOptions = {}, parseFn: ParseFn = parse):  Function {
  // Do whatever you need with the options

  // Will return a simple function which takes a text, but uses the previously provided emojiMap context
  return (text) => parseFn(emojiMap, options, text);
}

Usage:

import { createEmojiParse, fetchGithubEmojis } from 'gh-emoji';

const text = 'Do you believe in :alien:...? :scream:';

fetchGithubEmojis().then(emojiMap => {
  const options = {};
  const parse = createEmojiParse(emojiMap, options);

  const parsedText = parse(text);

   updatePreview(parsedText)
});

// Or without any github emoji fetching
const emojiMap = {
   alien: 'https://pics/alien.png',
   scream: 'https://pics/scream.png',
};

const parse = createEmojiParse(emojiMap, {});
parsedText = parse(text);
updatePreview(parsedText);

BONUS: Now, if we wanna design plugins, we can just stick to our ParseFn definition:

import { parse, createEmojiParse } from 'gh-emoji';

const emojiMap = {
   alien: 'https://pics/alien.png',
   scream: 'https://pics/scream.png',
};

const text = 'My :god:';

// ---- START LIB CODE -----
// Will replace each :emoji: with twice the number of emojis
// like :emoji: => :emoji::emoji:
const doubleEmojisPlugin = (parseFn) => (emojiMap, options, text) => {
       const doubleEmojiText = someDoubleTransform(text);
       return parseFn(emojiMap, options, text);
}
const applyPlugin = (parseFn, ...plugins) => compose(parseFn, plugins);
// ---- END LIB CODE -----

const enhancedParse = applyPlugin(parse, doubleEmojisPlugin);

const emojiParse = createEmojiParse(emojiMap, {}, enhancedParse);

// Will now render something like "My :god::god:"
const parsedText = emojiParse(text);
updatePreview(parsedText);
rpunkfu commented 8 years ago

From now on issues about changing API / splitting into modules shall go into #27