mui / material-ui

Material UI: Ready-to-use foundational React components, free forever. It includes Material UI, which implements Google's Material Design.
https://mui.com/material-ui/
MIT License
91.73k stars 31.51k forks source link

[code-infra] Replace all references of event as e #41968

Open Janpot opened 2 weeks ago

Janpot commented 2 weeks ago

For example https://github.com/mui/material-ui/blob/1103320f310e799216d6f2cf76d4e8054b1dc0de/docs/data/joy/components/list/ExampleNavigationMenu.tsx#L67

Also enforce this with an eslint rule. e.g. ~https://eslint.org/docs/latest/rules/id-length~ https://eslint.org/docs/latest/rules/id-denylist

Single letter identifiers are desired in some cases (t for translations, and i, j, k for loop variables). So let's keep the scope of this issue down to specific identifiers only and not enforce it by length. Some other ids I can think of are:

danilo-leal commented 2 weeks ago

I've also seen somewhat similar stuff with t being used for translation:

https://github.com/mui/material-ui/blob/657705633bbcf85019aa8bbf16d143a0d304edfe/docs/data/material/components/material-icons/SearchIcons.js#L263

Should we think of it similarly?

Janpot commented 2 weeks ago

Maybe, it's up to the core team IMO. I think we're all quite on the same page for e=>event

I found this issue back in the GitHub project I'm trying to revive for code-infra. Maybe we should groom this a bit first

flaviendelangle commented 1 week ago

The one-letter variables are a never-ending topic in software development :laughing:

For e => event, it enforces some consistency across the codebase so I'm all in favor of doing it.

IMHO the rule should not apply to for loops for (let i=0), and for map / filter it's debatable but I don't have a strong preference.

Janpot commented 1 week ago

I agree. Ok, I propose then to keep the scope of this issue to e => event only. And to instead use the id-denylist rule. wdyt?

AfaqShuaib09 commented 6 days ago

Hey, Could someone this issue to me? I would love to make a PR on this issue Thanks

Janpot commented 5 days ago

@AfaqShuaib09 Go for it