reselect: Error with composed selector

I’m creating a composed selector with a simple selector from another file and I get this error.

Uncaught Error: Selector creators expect all input-selectors to be functions, instead received the following types: [undefined]

The selector I am importing from locationsSelectors.js is:

export const getAllLocations = state => state.locations;

And here is how I am trying to use it:

import {createSelector} from 'reselect';
import {getAllLocations} from './locationSelectors';

export const myNewSelector = createSelector(
    [getAllLocations],
    (locations) => {
        return 'foo';
    }
);

This is a simplified example to demonstrate the problem. The weird thing is that if I move the imported getAllLocations selector into this file, it works fine. Another weird thing is that I can import selectors for composition from other files as well, but only some of them.

About this issue

  • Original URL
  • State: closed
  • Created 8 years ago
  • Comments: 35 (4 by maintainers)

Most upvoted comments

As it turns out, this is a webpack/ES6 module circular dependency issue. The value of getAllLocations eventually resolves correctly, but it is undefined at first which causes reselect to throw the error. Outside of moving the shared code, is there a way to address this in reselect? Details here: http://stackoverflow.com/questions/35240716/webpack-import-returns-undefined-depending-on-the-order-of-imports

Circular dependencies between selectors are not necessarily a code architecture issue. Consider the following contrived blog-ish example:

// posts.js
import getCurrentUser from './users';

export const getPosts = (state) => state.posts;

/**
 * @param {Object} state
 */
export const getPostsByCurrentUser = createSelector(
  getPosts,
  getCurrentUser,
  (posts, user) => posts.filter(({ userId }) => userId === user.id)
);
// users.js
import getPosts from './posts';

export const getCurrentUser = (state) => state.users.current;

/**
 * @param {Object} state
 * @param {String} userId
 */
export const getUserPostCount = createSelector(
  getPosts,
  (_, userId) => userId
  (posts, userId) => posts.filter(({ userId }) => userId === user.id).length
);

There is nothing explicitly wrong w/ having a circular dependency between posts.js and users.js. AFAIK, the circular dependency === bad logic applies mostly to OOP situations, e.g. circular dependencies bt/ classes. But in functional situations in general, circular dependencies between functions aren’t really an anti pattern (it’s required for mutual recursion), and I don’t think the specific case of circular dependencies across modules would necessarily be any different.

Given that a selector might not be available at module evaluation (when the selector is created) should not matter, given that the selector is not invoked until later, by which point the reference should have resolved. For this reason, circular dependencies bt/ functions are generally not an issue, assuming that the functions are not invoked before all modules have been evaluated.

However, b/c webpack (and afaik all other bundlers) passes arguments to createSelector as values, not as bindings, they do not end up pointing to the correct selector function after the dependent module is evaluated, and are still undefined when the selector is invoked.

es6 module bindings are supposed to handle this case correctly, but afaik, no bundler build tool gets this case right. So, :shrug-emoji, until one does, I guess the only workaround is to ensure that selectors don’t contain circular dependencies (even if they arguably should be allowed to do so)…

FYI, here is a README I added to our codebase that explains pretty much everything discussed above. Thanks for your help folks 🙇‍♂️

Reselect Circular Dependencies

We use the reselect library to create memoized selectors.

How the circular dependencies happen

Imagine the following hypothetical situation: In posts.js

// posts.js
import getCurrentUser from "./users";
export const getPosts = state => state.posts;
export const getPostsByCurrentUser = createSelector(
  getPosts,
  getCurrentUser,
  (posts, user) => posts.filter(({ userId }) => userId === user.id)
);

In users.js:

// users.js
import getPosts from './posts';
export const getCurrentUser = (state) => state.users.current;
export const getUserPostCount = createSelector(
  getPosts,
  (_, userId) => userId
  (posts, userId) => posts.filter(({ userId }) => userId === user.id).length
);

We now have a circular dependency because posts.js requires users.js, and users.js requires posts.js

Mermaid Diagram

How the problem appears to an engineer

You will end up with an error like:

    Selector creators expect all input-selectors to be functions, instead received the following types: [undefined, function, function]
      356 | );
      357 |
    > 358 | export const getUserPostCount = createSelector(
          |                                         ^
      359 |   [
      360 |     getPosts,
      361 |     (_, userId) => userId,

In the code snippet above, the first input-selector getPosts was not defined when the user.js module was evaluated. This is why reselect thinks getPosts is undefined.

Possible workarounds

1. Pass in an anonymous function that calls your input-selector

Currently, the following event sequence is occurring:

  1. user.js module is being resolved
  2. createSelector is called. It tries to call/invoke getPosts. But posts.js has not been resolved yet. So we get a “Selector creators expect all input-selectors to be functions” error.

If we update getUserPostCount to:

// user.js
export const getUserPostCount = createSelector(
  (state) => getPosts(state), // anonymous function that calls input-selector.
  (_, userId) => userId
  (posts, userId) => posts.filter(({ userId }) => userId === user.id).length
);

Then a different sequence of events will happen:

  1. user.js module is being resolved
  2. createSelector is called. The first argument holds a reference to getPosts i.e. getPosts does not get called!
  3. posts.js module is resolved
  4. When getUserPostCount is invoked, then getPosts is called which is fine because the posts.js module has already been resolved.

This trick doesn’t get rid of the circular dependency. posts.js still imports user.js and user.js still imports posts.js. The trick just prevents getPosts getting invoked before it is defined

2. Create a new selector file

Re-arrange files so that the problematic circular dependency is no longer there. In our example:

Remove any logic pertaining to a user’s posts from posts.js:

// posts.js
export const getPosts = state => state.posts;

Remove any logic pertaining to a user’s posts from user.js:

// users.js
export const getCurrentUser = state => state.users.current;

Create a new userPosts.js selector file

// userPosts.js
import getCurrentUser from './users';
import getPosts from './posts';
export const getPostsByCurrentUser = createSelector(
  getPosts,
  getCurrentUser,
  (posts, user) => posts.filter(({ userId }) => userId === user.id)
);
export const getUserPostCount = createSelector(
  getPosts,
  (_, userId) => userId
  (posts, userId) => posts.filter(({ userId }) => userId === user.id).length
);

Mermaid Diagram 2

Unlike Workaround 1, this removes the circular dependency all together. Remember if you move the functions around, you should also relocate their tests.

Just spend all day dealing with this too!! Ugh!. Wish there was a warning about this in the readme… since it’s obvious that unless all your selectors are in the same file (they shouldn’t be) you will run into this issue when composing selectors.

I stumble over this issue and immediately came up with simple solution (based on @jameslaneconkling example):

// posts.js

export const getPostsByCurrentUser = createSelector(
  getPosts,
  (state, props) => getCurrentUser(state, props),
  (posts, user) => posts.filter(({ userId }) => userId === user.id)
);

export const getUserPostCount = createSelector(
  (state, props) => getPosts(state, props),
  (_, userId) => userId
  (posts, userId) => posts.filter(({ userId }) => userId === user.id).length
);

Maybe it will be helpful for somebody if they really don’t want to start reorganising their whole project structure. Does anybody see downsides of this approach?

Super frustrating. We’ve been battling with this for a while. This is often related to circular dependencies. When we run into this issue we use madge to detect circular dependencies.

When there’s a circ dependency, we have to evaluate what is causing it. Usually, the fix involves moving a couple selectors into another file where they make more sense. Otherwise, we pull them into a core folder. Also all of our top level selectors (like const getFoos = (state) => state.foos) live in this global folder.

This issue isn’t specific to reselect. Any library that composes many dependencies at initialize-time will have similar problems. As @stevenmusumeche indicated

this is a webpack/ES6 module circular dependency issue

this is a webpack/ES6 module circular dependency issue

This is not a Webpack or a Reselect problem at all. Your code is not de-coupled correctly. If you have issues where code is ultimately depending on itself through a circular dependency, you need to look at breaking apart your code into separate files/responsibilities.

That being said, there are times when I have specifically UI-driven selectors depending on purely data-driven selectors but you have to be extremely careful when doing this and understand your structure very well. And even then, I’m questioning whether it’s the right approach.

A workaround which worked for me was to define the imported selector using standard function syntax, e.g. change:

export const getIsLoading = state => state.isLoading

to:

export function getIsLoading(state) { return state.isLoading }

But this is annoying as arrow syntax is more ideal.

I stumbled upon a similar issue when testing selectors. However, my issue was completely independent of circular dependencies.

If you depend on an external selector module and you’re mocking said module, make sure you actually write a mock selector for the selector you depend on. Don’t be like me and get an undefined function error just because you forgot to write a mock selector.

@giacomocerquone : I’ve got links to a bunch of articles on selectors and architecture (including that one) here:

https://github.com/markerikson/react-redux-links/blob/master/redux-architecture.md#encapsulation-and-reusability

@shenders13 thanks for posting that, it’s a good summary of the workarounds. Did you do any testing to confirm whether workaround #1 has any implications worth noting? For example, whether it affects memoization in some way, and as a result has a negative effect on performance?

@shenders13 is the issue of circular dependencies not solved by @daniula 's above solution?

I work on a codebase with 50+ selector files. The large number of selector files together with the fact that it’s common for there to be 10 or more selectors in a single selector file means we’re running into circular dependency hell more and more as time goes on.

A common workaround I’ve seen people do is to create a new selector file whenever you create a new selector that accidentally spawns circular dependency hell. This is probably a contributing factor to why we have so many selector files in the first place.

It seems like moving all selectors into one giant selector file would fix the problem.

Pro’s

  • We won’t have this issue anymore. This will improve developer velocity. It took me about 2-3 hours to understand how to implement “work around” for a circular dependency last sprint.

Con’s

  • Refactor will be a pain
  • Increase the frequency of merge conflicts as the master selector file will get updated a lot.
  • The file may get so big it will cause IDE’s to lag.

Are there any other pro’s/con’s I’m missing?

recompose determines whether to use a selector’s memoized result by comparing the result of all inputSelectors. So, in the above case, as long as getPosts returns a result that is equal across calls, (state, props) => getPosts(state, props) will also return the same result, and the memoized value will be returned.

That’s how I understand it. Please correct me if I’m wrong.

@daniula wow, yeah, of course. simply delay the resolution of the selector (getPosts, in the above case) until the function is called, rather than when the module is first evaluated. Good thinking–thanks!

In my case, I had declared createSelector before selectors functions (in one file). Propably, in your case, getAllLocations, somehow undefined yet.

The error is saying that getAllLocations is undefined, so I guess something is wrong with the export from locationSelectors. I’m going to close this as it doesn’t appear to be a Reselect issue, but if you give me a link to some example code that I can run that demonstrates the problem I’ll see if I can figure whats happening.