reselect icon indicating copy to clipboard operation
reselect copied to clipboard

Better call stack for selector warnings

Open uncaught opened this issue 1 year ago • 11 comments

The call stack for the new dev mode checks starts/ends with useSelector. But if you have a lot of nested selectors, finding the one actually causing the warning can be a bit tedious.

Would it be possible to add the location of the actual createSelector-call to the warning?

E.g.:

//file a
const a = createSeletctor(...);

//file b
const b = createSeletctor(...); //causing the warning
const c = createSeletctor(...);

//file d
const d = createSeletctor(a, b, c, () => a+b+c /* not causing the warning */);

//file e
useSelector(d); //generates the warning, call stack states this line only

-> Please add "file b, line x" to the warning.

uncaught avatar Dec 18 '23 08:12 uncaught

@uncaught Yeah I just looked at it again and you're right, the location where the selector is defined is not in the stack trace, I'm going to look into this. Thanks for opening the issue.

aryaemami59 avatar Dec 18 '23 09:12 aryaemami59

I'm not convinced we can do much about this? we get the stack string by throwing and catching an Error, it's up to the platform what the stack actually looks like

EskiMojo14 avatar Jan 15 '24 18:01 EskiMojo14

Well yes, but you don't need to hack the system's callstack to show something in the warning message.

Like if you determine the file and location of the createSelector call, store it and print it out with the warning.

uncaught avatar Jan 15 '24 19:01 uncaught

@uncaught the issue is that trying to parse the stack and come up with the filename and such is more complex and browser-dependent.

It's not impossible. But it would definitely take extra code and extra effort, and reading a stack trace is a core skill that we would expect end user devs to have.

markerikson avatar Jan 15 '24 20:01 markerikson

Haven't tried it, I'm on my phone, but this I've just found in a minute:

(new Error()).stack.split("\n")[1].split("/").slice(-1)[0]

Store this and print it in the error handler.

The issue here is that the call stack doesn't show the location of the faulty selector at all. This is not about being too lazy to read it.

uncaught avatar Jan 15 '24 20:01 uncaught

if it's not in the call stack, i don't see how we'd get that information for you?

EskiMojo14 avatar Jan 15 '24 20:01 EskiMojo14

As i said, determine the file+line of the createSelector call, which should be possible, see above. Then store that file and line number somewhere. For example put it on the resolver function as a Smybol or create a Map for it or whatever.

Then when you print that warning, look up the file an number for the selector that caused the warning situation and print it.

uncaught avatar Jan 15 '24 20:01 uncaught

We're open to PRs here, but the current system does show useful information as-is and this isn't an immediate priority for us.

markerikson avatar Jan 15 '24 20:01 markerikson

I thought @aryaemami59 was already looking into it (first comment).

But sure, this isn't a high priority thing, just an inconvenience.

uncaught avatar Jan 15 '24 20:01 uncaught

@uncaught I don't think we're going to try and parse the stack, for the reasons Mark stated.

With that said, it's doable to create a stack when you first call createSelector - do you fancy trying the build from https://github.com/reduxjs/reselect/pull/684 and seeing if that includes the information you'd want?

EskiMojo14 avatar Jan 15 '24 22:01 EskiMojo14

@uncaught sorry I got busy with other things, I did play around with it some time ago, it seemed like it was going to be more challenging than I anticipated, so it was in my todo list just didn't get around to dealing with it yet. And it seems like @EskiMojo14 is going to give it a shot, he is much more quialified to look at this than I am anyway.

aryaemami59 avatar Jan 16 '24 00:01 aryaemami59