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

Make Collection.prototype.get fail gracefully when __paths.length === 0 #140

Closed
DrewML opened this issue Aug 11, 2016 · 2 comments
Closed

Comments

@DrewML
Copy link
Contributor

DrewML commented Aug 11, 2016

I spent more time debugging this one tonight than I'd like to admit :). Minimal Repro is here.

When calling .get on a collection without any paths, an exception is thrown. Unfortunately, because the exception message itself references get, I made the (poor) assumption that it was my call to get, not the implementation's call to path.get.

Would you be willing to accept a PR to change the behavior here? I think either returning a falsy value (maybe null, similar to document.querySelector) or throwing a more specific error (encouraging the user to check Collection.prototype.size first) would work.

@fkling
Copy link
Contributor

fkling commented Aug 12, 2016

I would certainly accept a pull request to improve this 😃 .

Which behavior would you prefer? I feel like throwing an error might be more helpful?

@DrewML
Copy link
Contributor Author

DrewML commented Aug 12, 2016

I feel like throwing an error might be more helpful?

Yeah, I think you're right. Returning something would probably still lead to less-than-helpful failures anyway, since code will likely chain off of get.

I'll submit a PR for that change.

@fkling fkling closed this as completed in 0de6d3c Aug 15, 2016
fkling added a commit that referenced this issue Aug 15, 2016
Throw helpful error when calling Collection.prototype.get with 0 paths (fixes #140)
euphocat pushed a commit to euphocat/jscodeshift that referenced this issue Oct 22, 2017
Adds PureComponent support to ReactUtils
euphocat pushed a commit to euphocat/jscodeshift that referenced this issue Oct 22, 2017
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

2 participants