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

Feature: js object shorthand #194

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

scr2em
Copy link
Contributor

@scr2em scr2em commented Apr 14, 2024

.grit/patterns/js/no_implicit_coercion.md Outdated Show resolved Hide resolved
.grit/patterns/js/no_implicit_coercion.md Outdated Show resolved Hide resolved
.grit/patterns/js/no_implicit_coercion.md Outdated Show resolved Hide resolved
title: Disallow shorthand type conversions
tags: [good-practice]
---
This matches the [eslint rule](https://eslint.org/docs/latest/rules/no-implicit-coercion).
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a lot more logic from the eslint rule that would be needed to make this correct. It's probably not useful tbh.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@morgante I found it very useful for readability. I ran it on an internal project +200 matches, it went smoothly. can you give an example ?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you look in the eslint source it does more checks to ensure things like booleans aren't doubly cast. If you want to take the time to mirror that logic, I don't object - but most people will probably still just use eslint or biome.

Copy link
Contributor Author

@scr2em scr2em Apr 14, 2024

Choose a reason for hiding this comment

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

@morgante I actually just noticed I took the branch from the other feature (no implicit coercion). I'll split the PR. I'll also check the ESlint source, but probably gonna look for another rule

.grit/patterns/js/object_shorthand.md Outdated Show resolved Hide resolved
.grit/patterns/js/object_shorthand.md Outdated Show resolved Hide resolved
.grit/patterns/js/object_shorthand.md Outdated Show resolved Hide resolved
.grit/patterns/js/object_shorthand.md Outdated Show resolved Hide resolved
@scr2em scr2em force-pushed the Feature/js-object-shorthand branch from 0501826 to ca29cd9 Compare April 14, 2024 18:44
@scr2em scr2em force-pushed the Feature/js-object-shorthand branch from 4f27da6 to 981bfce Compare April 14, 2024 19:01
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

2 participants