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

Rule proposal: text-encoding-identifier-case #1690

Closed
fisker opened this issue Jan 11, 2022 · 21 comments · Fixed by #1718
Closed

Rule proposal: text-encoding-identifier-case #1690

fisker opened this issue Jan 11, 2022 · 21 comments · Fixed by #1718
Assignees
@fisker
Labels
help wanted new rule

Comments

@fisker
Copy link
Collaborator

fisker commented Jan 11, 2022 •
edited

Description

Enforce consistent style of "UTF-8" encoding.

In many repos, 'utf8' and 'utf-8' both used. I'm not sure what's the best choice, I believe UTF-8 is the standard name, but Node.js docs recommended utf8 in many places. Maybe configurable?

Fail

fs.readFile(foo, 'utf-8');
new TextDecoder('UTF-8');

Pass

fs.readFile(foo, 'utf8');
new TextDecoder('utf8');
@sindresorhus
Copy link
Owner

I prefer utf8. This is what most Node.js users would use too. Yes, UTF-8 is the official name, but in this context, it's used more like an identifier / enum case. JavaScript doesn't have enums, but we instead use string unions. The conventions for these is that they're formatted as if they were proper identifiers.

@sindresorhus
Copy link
Owner

I also think the rule name should be utf8-encoding-case.

@fisker
Copy link
Collaborator Author

fisker commented Jan 11, 2022

I'm fine with utf8, main point is to avoid different cases.

@fisker fisker changed the title Rule proposal: utf-8-encoding-case Rule proposal: utf8-encoding-case Jan 11, 2022
@silverwind
Copy link
Contributor

Does Node.js accept UTF-8 for encoding?

@silverwind
Copy link
Contributor

For reference, here are the spec details on the TextEncoder argument:

https://encoding.spec.whatwg.org/#concept-encoding-get

@silverwind
Copy link
Contributor

I think the rule should be called encoding-case to support other encodings besides UTF-8 as well.

@sindresorhus
Copy link
Owner

I don't really see the point of complicating the rule with any possible encoding. The 99.99% case is UTF-8.

@silverwind
Copy link
Contributor

There are still many cases where one has to deal with ascii encoding, especially when dealing with network protocols like DNS in Node.js. Those protocols will never get UTF-8 support for compatibility reasons. I think it's short-sighted to only support one encoding.

@papb
Copy link

papb commented Jan 18, 2022

So only utf8 and ascii, then?

@sindresorhus
Copy link
Owner

I'm fine with including ascii too.

@fisker
Copy link
Collaborator Author

fisker commented Jan 18, 2022

My plan is to report all string literals that looks like a 'utf8' or 'ascii' encoding, no matter where it's used.

@fisker
Copy link
Collaborator Author

fisker commented Jan 25, 2022 •
edited

encoding-case? charset-case?

@sindresorhus
Copy link
Owner

I would go with text-encoding-identifier-case.

@sindresorhus
Copy link
Owner

Accepted

@fisker fisker changed the title Rule proposal: utf8-encoding-case Rule proposal: text-encoding-identifier-case Feb 7, 2022
@fisker fisker self-assigned this Feb 7, 2022
@fisker fisker mentioned this issue Feb 9, 2022
Merged
@fisker
Copy link
Collaborator Author

fisker commented Feb 9, 2022 •
edited

I found something in the WHATWG spec, I hope use utf8 on the web won't cause problems, https://encoding.spec.whatwg.org/#names-and-labels

Authors must use the UTF-8 encoding and must use the ASCII case-insensitive "utf-8" label to identify it.

@domoritz
Copy link

domoritz commented Feb 20, 2022 •
edited

I am getting false positives in my jest test definitions with this rule:

describe('Utf8', () => { validateVector(generate.utf8()); });

It would probably be good to only apply this rule for specific function calls.

@pcorpet
Copy link

pcorpet commented Mar 1, 2022

According to WhatWG, in HTML spec the <meta charset> tag must be an ASCII case-insensitive match for the string "utf-8".

This is causing conflicts in our React code (in .tsx files that are linted by eslint):

  return <html lang="en">
    <head>
      <link rel="icon" type="image/x-icon" href={favicon} />
      <meta charSet="utf-8" />

@Primajin
Copy link

Yeah I'm facing the same issue like @pcorpet - is there a way to make the rule aware to not apply this to JSX / meta charset?

@fisker
Copy link
Collaborator Author

fisker commented May 21, 2022

I'll fix that.

@fisker fisker mentioned this issue May 21, 2022
Merged
@fisker
Copy link
Collaborator Author

fisker commented May 21, 2022

#1817

@Primajin
Copy link

#1817

Thanks a bunch!

@fisker fisker mentioned this issue Oct 26, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fisker fisker

Labels
help wanted new rule
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add text-encoding-identifier-case rule fisker/eslint-plugin-unicorn
7 participants
@silverwind @sindresorhus @fisker @domoritz @Primajin @pcorpet @papb