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)
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:
There is nothing explicitly wrong w/ having a circular dependency between
posts.js
andusers.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
In
users.js
:We now have a circular dependency because
posts.js
requiresusers.js
, andusers.js
requiresposts.js
How the problem appears to an engineer
You will end up with an error like:
In the code snippet above, the first input-selector
getPosts
was not defined when theuser.js
module was evaluated. This is why reselect thinksgetPosts
is undefined.Possible workarounds
1. Pass in an anonymous function that calls your input-selector
Currently, the following event sequence is occurring:
user.js
module is being resolvedcreateSelector
is called. It tries to call/invokegetPosts
. Butposts.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:Then a different sequence of events will happen:
user.js
module is being resolvedcreateSelector
is called. The first argument holds a reference togetPosts
i.e.getPosts
does not get called!posts.js
module is resolvedgetUserPostCount
is invoked, thengetPosts
is called which is fine because theposts.js
module has already been resolved.This trick doesn’t get rid of the circular dependency.
posts.js
still importsuser.js
anduser.js
still importsposts.js
. The trick just preventsgetPosts
getting invoked before it is defined2. 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
:Remove any logic pertaining to a user’s posts from
user.js
:Create a new
userPosts.js
selector fileUnlike 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):
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 (likeconst 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 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
Con’s
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 fromlocationSelectors
. 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.