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

Require Dot Notation Whenever Possible (dot-notation) #1344

Closed
julien-f opened this issue Feb 18, 2019 · 10 comments

Comments

@julien-f
Copy link

commented Feb 18, 2019

Hey folks, what do you think about enabling the dot-notation rule?

@LinusU

This comment has been minimized.

Copy link
Member

commented Feb 19, 2019

Generally positive, although I prefer using the [...] syntax when I have an object that's being used as a map. Now, in most modern code it would just be a Map instance and I wouldn't have this problem, but process.env comes to mind as somewhere were this would annoy me 🤔

@brodybits

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2019

process.env comes to mind as somewhere were this would annoy me

Couldn’t we solve this by using a utility such as process-env to access the process.env members in a more flexible manner?

And couldn’t we use a similar wrapper solution in other cases someone needs […] member access?

I suspect minor efficiency cost should not prove to be a real issue given the improved readability in what seem to be exceptional cases.

@brodybits

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2019

I would love to see this considered for Standard 14 (#1321).

@feross feross transferred this issue from standard/eslint-config-standard Jul 28, 2019

@feross

This comment has been minimized.

Copy link
Member

commented Jul 28, 2019

Modest ecosystem impact:

1..162
# tests 162
# pass  153
# fail  9

And they're all cases where dot notation would be a bit better. Since the rule is automatically fixable, I think this rule is a go for standard v14.

@feross feross added this to the standard v14 milestone Jul 28, 2019

feross added a commit to standard/standard-packages that referenced this issue Jul 28, 2019

@feross feross changed the title Favor dot notation for property access Require Dot Notation Whenever Possible (dot-notation) Jul 28, 2019

@feross

This comment has been minimized.

Copy link
Member

commented Jul 28, 2019

After fixing repos I control, only 3 ecosystem repos fail.

  • electron-spawn
  • cliui
  • application-config-path

@feross feross closed this in 5e3161b Jul 28, 2019

@LinusU

This comment has been minimized.

Copy link
Member

commented Jul 29, 2019

application-config-path is mine, I can fix it 👍

Funnily enough, it's exactly the case I described in my first comment and I actually think that the code will be a bit uglier 😬

Oh well, it's for the greater good 🚀

@feross

This comment has been minimized.

Copy link
Member

commented Jul 29, 2019

Yeah, I actually agree with you. There's a small amount information that it feels like we're losing by always using dot notation. I noticed when I went through all the failing cases that I was using [] consistently in very specific situations like HTTP headers: res.headers['accept']. It's hard to put into words why this way feels better than dot notation for these certain cases, but it does.

Happy to reconsider this if we end up not liking it. But it's only required changes in a few places so I'm guessing it won't be that big of a deal :)

@LinusU

This comment has been minimized.

Copy link
Member

commented Jul 29, 2019

[...] using the [...] syntax when I have an object that's being used as a map.

I think this is what does it for me. JavaScript doesn't really have a distinction between objects with specific properties, and just general maps/dictionaries (or, it has now Map, but many APIs came before that. You can see this clearly with TypeScript typing: process.env and res.headers are described as Record<string, ...> whereas objects have there specific keys described.

It would be awesome if it could somehow be enforced that process.env, req.headers, etc. always have [...] notation, but I think that's impossible 😄

@vweevers

This comment has been minimized.

Copy link

commented Aug 23, 2019

It's hard to put into words why this way feels better than dot notation for these certain cases, but it does.

In the case of headers, it feels better because you may have (or later add) headers that do require [...] syntax. For visual consistency, I prefer:

headers['user-agent'] = 'example'
headers['connection'] = 'keep-alive'
headers['accept-encoding'] = 'gzip, deflate, identity'

Over:

headers['user-agent'] = 'example'
headers.connection = 'keep-alive'
headers['accept-encoding'] = 'gzip, deflate, identity'
@mightyiam

This comment has been minimized.

Copy link
Contributor

commented Aug 23, 2019

Perhaps I would write that way, as well, @vweevers. If you could contribute that as an option to the ESLint rule, then we'd have the option to consider it. I personally feel that it would be better to choose a not-always pleasing style over not choosing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
You can’t perform that action at this time.