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

Add throw new Error('') snippet #38

Open
piotrwitek opened this issue Aug 10, 2016 · 5 comments
Open

Add throw new Error('') snippet #38

piotrwitek opened this issue Aug 10, 2016 · 5 comments

Comments

@piotrwitek
Copy link

Hello,
I'm missing throw new snippets could you please add ones for following use cases:

  • throw new Error('${message}); -> tne
  • throw new ${ErrorType()}; -> tnet
@extrabacon
Copy link
Owner

How about te for throw Error(...) and tne for throw new ErrorType(...)?

The new is necessary only for subclassed errors.

@piotrwitek
Copy link
Author

piotrwitek commented Aug 10, 2016

Hi @extrabacon thanks for quick response :)
Regarding new I cannot agree.
The proper use of creating a new Error object should always be used with new keyword.
Reference: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error
Please also check examples:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error#Error_types

I think what you are referring to is a generic throw expression;
Reference: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/throw

If you have some rationale for your side please reference them, from my perspective not using new is a bad practice and this great extension shouldn't propagate them.

@extrabacon
Copy link
Owner

Yes, we should use new for Error, but I'm lazy and turbo javascript is all about speed.

Also, it's definitely safe. All built-in error constructors can be used as functions.
http://stackoverflow.com/questions/13294658/throw-errormsg-vs-throw-new-errormsg

However, we cannot say the same for subclassed errors, as there is no guarantee that its constructor can be used as a function (unless the implementor added this behavior). Thus, it's only needed in this case. Bonus: it makes it really easy to spot custom errors VS builtin errors, just by using the new operator.

In terms of snippets, tne could have Error as its default value, supporting both scenarios. Just don't use te if you dislike it.

What do you think?

@piotrwitek
Copy link
Author

@extrabacon Thanks for explanation you are right that you can use Error without new as it behaves as factory function. This is really fine with me, my concern is that I think they can promote practice that lack consistency, and because of the you could later make silly errors like e.g. forget to add new for subclassed errors. I'm using an eslint rule to always guard me from this kind of bugs.

Regarding snippets I think they look great :)

Thanks a lot!

@extrabacon
Copy link
Owner

Using linting is definitely a must if you wish to catch such issues. I will go forward with adding the snippets.

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