reselect icon indicating copy to clipboard operation
reselect copied to clipboard

Consider dev mode checks for `x => x` result functions

Open markerikson opened this issue 2 years ago • 1 comments

Just ran across these lovely functions in the wild:

  • https://github.com/flipt-io/flipt/issues/2444
export const selectFlag = createSelector(
  [
    (state: RootState, namespaceKey: string, key: string) => {
      const flags = state.flags.flags[namespaceKey];
      if (flags) {
        return flags[key] || ({} as IFlag);
      }

      return {} as IFlag;
    }
  ],
  (flag) => flag
);

export const selectNamespaces = createSelector(
  [
    (state: RootState) =>
      Object.entries(state.namespaces.namespaces).map(
        ([_, value]) => value
      ) as INamespace[]
  ],
  (namespaces) => namespaces
);

export const selectCurrentNamespace = createSelector(
  [
    (state: RootState) => {
      if (state.namespaces.namespaces[state.namespaces.currentNamespace]) {
        return state.namespaces.namespaces[state.namespaces.currentNamespace];
      }
      return { key: 'default', name: 'Default', description: '' } as INamespace;
    }
  ],
  (currentNamespace) => currentNamespace
);

These are broken in two ways:

  • input selectors returning new references
  • result functions that are x => x

We've got warnings for the first, but not for the second.

Here's another example:

export const helmChartsSelector = createSelector(
  (state: RootState) => state.main.helmChartMap,
  helmCharts => helmCharts
);

This isn't necessary and shouldn't even be memoized.

A really hacky thing to do would be:

  • Stringify the result function
  • Parse its arguments and body
  • check if args.length === 1 && args[0] === body (conceptually. probably multiple ways to do this, ranging from actual parsing to a regex or something)

I'm actually seriously considering us adding this

Possible resources:

  • https://github.com/sindresorhus/fn-args/blob/main/index.js
  • https://stackoverflow.com/questions/38453881/how-to-get-an-arrow-functions-body-as-a-string
  • https://stackoverflow.com/questions/6921588/is-it-possible-to-reflect-the-arguments-of-a-javascript-function
  • https://stackoverflow.com/questions/3179861/javascript-get-function-body

markerikson avatar Nov 25 '23 18:11 markerikson

Even better, Lenz just came up with this nonsense:

function checkErroneousResultFunction(resultFn){
  let wtf = false
  try {
    const obj = {}
    if (resultFn(obj) === obj) wtf = true
  } catch {}
  if (wtf) {
    throw new Error("RTFM")
  }
}

markerikson avatar Nov 25 '23 19:11 markerikson

I believe this one has been resolved by #645.

aryaemami59 avatar Mar 16 '24 18:03 aryaemami59