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

Description of nonew does not account for ES6 classes #2910

Open
fluky opened this issue Apr 7, 2016 · 4 comments
Open

Description of nonew does not account for ES6 classes #2910

fluky opened this issue Apr 7, 2016 · 4 comments

Comments

@fluky
Copy link

fluky commented Apr 7, 2016

The docs state:

This option prohibits the use of constructor functions for side-effects. Some people like to call constructor functions without assigning its result to any variable:

new MyConstructor();
There is no advantage in this approach over simply calling MyConstructor since the object that the operator new creates isn't used anywhere so you should generally avoid constructors like this one.

This is not correct when using ES6 classes, which cannot be instantiated without new. Is there a possibility of updating this rule to detect a class (in the same module) and ignore the rule for this case?

@caitp
Copy link
Member

caitp commented Apr 7, 2016

It's hard for a static analysis tool to even figure out if what you're trying to construct is a class or not --- there would definitely be false positives, which isn't ideal.

Why not just not use "nonew" when dealing with ES6 code? --- A docs update might be in order to clarify this

@caitp
Copy link
Member

caitp commented Apr 7, 2016

Also, there has been some discussion about making classes callable by some mechanism --- @rwaldron might know if there was any more about that last week.

@fluky
Copy link
Author

fluky commented Apr 7, 2016

It's hard for a static analysis tool to even figure out if what you're trying to construct is a class or not --- there would definitely be false positives, which isn't ideal.

I agree. That that's why I said "in the same module". Obviously if you are looking at one file at a time, it would be very hard, if not impossible to to determine a function vs a class.

Why not just not use "nonew" when dealing with ES6 code?

That's my solution for now. I agree that at least a doc update is needed.

@jugglinmike jugglinmike changed the title Issue with ES6 and nonew Description of nonew does not account for ES6 classes Apr 7, 2016
@jugglinmike
Copy link
Member

This feature has previously been requested in gh-2579 and since implemented in
gh-2623. You'll note that the pull request has not been merged, largely for
the reason that @caitp described. If you feel that a warning that is only
occasionally reported is better than no warning at all, please feel free to
weigh in on that pull request.

Since this feature request has changed to a documentation issue, I've updated
the issue title to make the new intent a little more clear.

Also, there has been some discussion about making classes callable by some
mechanism --- @rwaldron might know if there was any more about that last
week.

There was more discussion! That proposal was withdrawn in favor of an approach
that uses the "decorators" proposal:
https://github.com/tc39/ecma262/blob/643a7ff106e3a1fd879e88b53675047502343f51/withdrawn-proposals.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants