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.

Sign up for GitHub

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

Jump to bottom

docs: Removed the second object instance for the "no-new" rule #17020

Merged
merged 2 commits into from Apr 2, 2023
Merged

docs: Removed the second object instance for the "no-new" rule #17020

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7041663
docs: Removed the second object instance for the "no-new" rule
sir-kain Mar 26, 2023
4027099
docs: re-define the function name
sir-kain Apr 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/src/rules/no-new.md
View file
Open in desktop
Expand Up @@ -43,7 +43,7 @@ Examples of **correct** code for this rule:

var thing = new Thing();

Thing();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @sir-kain, but I think this line was added intentionally to show that the rule does not report a problem if a constructor is called without new.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. In this case, would it be better to provide 2 separate examples to avoid confusion? (As we already do with lines-between-class-members rule)

For instance:

Examples of correct code for this rule:

/*eslint no-new: "error"*/

var thing = new Thing();

Examples of additional correct code for this rule:

/*eslint no-new: "error"*/

Thing();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. In this case, would it be better to provide 2 separate examples to avoid confusion? (As we already do with lines-between-class-members rule)

My feeling is that Thing(); alone does not make it immediately clear that a constructor is being invoked. This becomes evident when looking at the previous line. If the examples are split then there should be some additional explanation/context to clarify.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the best solution to avoid confusion, as these two examples are not related to each other, would be to rename Thing(); to something else. For example, Foo();.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the incorrect example, the class is named Thing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example aims to show that function calls are not reported even if the callee looks like a constructor, so Foo(); would be fine.

Copy link
Contributor Author

@sir-kain sir-kain Apr 2, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR updated.

Foo();
```

:::