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

using closures to refactor src/utils/options.js #170

Open
manpenaloza opened this issue Oct 17, 2018 · 1 comment
Open

using closures to refactor src/utils/options.js #170

manpenaloza opened this issue Oct 17, 2018 · 1 comment

Comments

@manpenaloza
Copy link
Member

Hi, just checked some code of the repo out of interest and saw the code of the src/utils/options.js file that looks like this (branch master, today):

function getOption({ opts }, name, defaultValue = true) {
  return opts[name] === undefined || opts[name] === null
    ? defaultValue
    : opts[name]
}

export const useDisplayName = state => getOption(state, 'displayName')
export const useSSR = state => getOption(state, 'ssr', true)
export const useFileName = state => getOption(state, 'fileName')
export const useMinify = state => getOption(state, 'minify')
export const useTranspileTemplateLiterals = state =>
  getOption(state, 'transpileTemplateLiterals')

export const usePureAnnotation = state => getOption(state, 'pure', false)

I saw the repetitive definition of exported functions that all got state as an argument which simply gets passed through to the execution of the getOption function. I tried to refactor this and came up with the upcoming code (which I haven't tested in terms of runtime functionality so far, but theoretically it should work).

const getOption = (name, defaultValue = true) => ({ opts }) => {
  return opts[name] === undefined || opts[name] === null
    ? defaultValue
    : opts[name] 
}

export const useDisplayName = getOption('displayName');
export const useSSR = getOption('ssr', true);
export const useFileName = getOption('fileName');
export const useMinify = getOption('minify');
export const useTranspileTemplateLiterals = getOption('transpileTemplateLiterals');
export const usePureAnnotation = getOption('pure', false);

... so the main difference would be that the getOption function returns a function that takes state as a parameter but has the proper option key preserved due to the usage of a closure. This would result to less function declarations in the code base and no need to pass through state down the execution chain.

Do you see a need for this? If yes, I can send a PR.

@mxstbr
Copy link
Member

mxstbr commented Oct 18, 2018

Feel free to send a PR, cleaner code is always better!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants