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

Consider dev mode checks for x => x result functions #642

Closed
markerikson opened this issue Nov 25, 2023 · 2 comments
Closed

Consider dev mode checks for x => x result functions #642

markerikson opened this issue Nov 25, 2023 · 2 comments

Comments

@markerikson
Copy link
Contributor

markerikson commented Nov 25, 2023

Just ran across these lovely functions in the wild:

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:

@markerikson
Copy link
Contributor Author

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")
  }
}

@aryaemami59
Copy link
Contributor

I believe this one has been resolved by #645.

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

3 participants