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

Disable more undesired syntax #207

Merged
merged 5 commits into from Sep 8, 2022
Merged

Disable more undesired syntax #207

merged 5 commits into from Sep 8, 2022

Conversation

rekmarks
Copy link
Member

@rekmarks rekmarks commented Sep 22, 2021

This PR expands the kinds of JavaScript syntax we forbid using the no-restricted-syntax rule to include:

  • with statements
    • Because with is very strange and we don't have a use case for it!
  • Function expressions
    • Because they're useless and weird! Just use arrow functions.
    • Note that a function expression is of the form const foo = function () { /* do something */ };, and is not to be confused with function declarations, e.g. function bar () { /* do something */ };
  • The in operator
    • Because the property lookup walks the prototype chain, wherefore e.g. 'toString' in {} evaluates to true. We should practically always use Reflect.hasOwnProperty instead.

@rekmarks rekmarks requested a review from a team as a code owner September 22, 2021 01:01
@Gudahtt
Copy link
Member

Gudahtt commented Sep 23, 2021

The instanceof operator, except if the right-hand operand is Error

I just thought of another use case for instanceof: custom error classes. e.g. https://github.com/MetaMask/auto-changelog/blob/94354653254e6c616efc9135c600c09961335740/src/cli.ts#L132

@mcmire
Copy link
Contributor

mcmire commented Apr 22, 2022

I think if we removed the instanceof rule for now then I would be good with this!

@Gudahtt
Copy link
Member

Gudahtt commented Apr 25, 2022

Agreed. Especially with TypeScript v4.4, I expect that instanceof with custom error classes will be a fairly common occurrence.

@rekmarks
Copy link
Member Author

@Gudahtt @mcmire I removed this:

  • The instanceof operator, except if the right-hand operand is Error
    • instanceof checks if the prototype of the two operands are the same. We can almost never be assured of this, except for globals like Error. We should just make explicit exceptions for this operator for those rare exceptions.

mcmire
mcmire previously approved these changes Aug 15, 2022
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

Sweet, looks good!

@Gudahtt
Copy link
Member

Gudahtt commented Aug 15, 2022

I am in favour of this as well. I can approve after the conflicts are resolved, and once v10 has been published. Better to keep this for the v11 release to avoid making the v10 update too challenging.

@rekmarks rekmarks merged commit 717b531 into main Sep 8, 2022
@rekmarks rekmarks deleted the restrict-more-syntax branch September 8, 2022 23:04
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

Successfully merging this pull request may close these issues.

None yet

3 participants