-
Notifications
You must be signed in to change notification settings - Fork 7
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
Log function info in env mapping error messages #14
Log function info in env mapping error messages #14
Conversation
1e8f703
to
e8b2e13
Compare
The use case for this was that I had updated a saga to include additional
I found it tedious to look at the args and then reference the updated saga to see what After the fix, that output was something like
|
That makes a lot of sense! I actually had the same issue myself, but wasn't sure of a way to improve the situation. Checking this out now. Excellent improvement @CoryDanielson! |
I pushed up a second commit so that the output matches node more. The output is more clear about the values being functions as well. In node REPL:
I edited my PR comment to include the new output. Anonymous functions will still have the definition output by |
tests/index.js
Outdated
t.throws( | ||
() => sagaTestEngine(f3, badMapping3), | ||
`Env Mapping is missing a value for ${JSON.stringify({ func: `[Function]: ${anonymousFunction.toString()}` }, null, 2)}` | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this fails on newer versions of node.
✔ redux-saga-test-engine (e8b2e13...) 🚀 nvm use 6.0.0
Now using node v6.0.0 (npm v3.8.6)
✔ redux-saga-test-engine (e8b2e13...) 🚀 yarn run test
yarn run v0.23.2
$ ava tests/
10 passed
✨ Done in 0.68s.
✔ redux-saga-test-engine (e8b2e13...) 🚀 nvm use 7.0.0
Now using node v7.0.0 (npm v3.10.8)
✔ redux-saga-test-engine (e8b2e13...) 🚀 yarn run test
yarn run v0.23.2
$ ava tests/
9 passed
1 failed
sagaTestEngine throws under bad conditions
/Users/tbuckley/dnainfo/redux-saga-test-engine/tests/index.js:190
189: const badMapping3 = [['also no', 'functions']]
190: t.throws(
191: () => sagaTestEngine(f3, badMapping3),
Env Mapping is missing a value for {
"func": "anonymousFunction"
}
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
The issue seems to be that function expressions like var anonymousFunction = function() { return '...' }
get assigned the variable name for their function name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmmmm interesting. I noticed that code run through Babel does the same thing. I might have to just remove the test unless I can think of something creative. Maybe conditionally skip the test if a function that is expected to be anonymous gets a name assigned to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're right that we can just remove or water down that particular test. The point is to provide as much information as possible to the user that the node engine allows.
- Anonymous function is skipped if node names the function.
ad5a016
to
584cf77
Compare
Okay, I've pushed up some changes and squashed my previous commits. The error messages for anonymous and named functions are now consistent. Both will output the I also did a check for the anonymous function |
This should be good to run through CircleCI again when you have time. |
Looks like it passed CI on master. I'll bump the version and deploy to npm soon. |
Problem
When a yielded value is not found in envMapping, the entire
val
is printed to the console viaJSON.stringify
, which, by default, does not output a key/value pair when the value is a function. Without information about the function being output in the error message, it's more difficult to track down what the missing mapping is.Fix
Updated the
JSON.stringify
call to use a custom replacer that outputs the function name or info about the anonymous function.