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

Use snake_case for components #779

Open
Archmonger opened this issue Jul 1, 2022 · 4 comments · May be fixed by #1052
Open

Use snake_case for components #779

Archmonger opened this issue Jul 1, 2022 · 4 comments · May be fixed by #1052
Labels
priority-3-low May be resolved one any timeline. type-docs About changes and updates to documentation

Comments

@Archmonger
Copy link
Contributor

Archmonger commented Jul 1, 2022

Current Situation

We currently promote using PascalCase for components, following ReactJS convention. However, this very much goes against PEP8.

See original discussion:

Proposed Actions

Rewrite the docs to promote snake_case usage.

@Archmonger Archmonger added type-feature About new capabilities type-docs About changes and updates to documentation priority-3-low May be resolved one any timeline. labels Jul 1, 2022
@Archmonger Archmonger added this to the 1.0 milestone Jul 1, 2022
@rmorshea
Copy link
Collaborator

We'll need to complete reactive-python/reactpy-flake8#9 in order to resolve this.

@Archmonger Archmonger removed the type-feature About new capabilities label Nov 9, 2022
@rmorshea rmorshea modified the milestones: 1.0, 2.0 Jan 12, 2023
@Archmonger Archmonger removed this from the Luxury Features milestone Jan 29, 2023
@Archmonger Archmonger linked a pull request Jun 14, 2023 that will close this issue
@JamesHutchison
Copy link

I actually prefer PascalCase because even though the implementation is a decorated function, you're using it like its a class.

@Archmonger
Copy link
Contributor Author

But this makes a lot of linters and IDEs not happy. We shouldn't have to make people modify their linting config to use ReactPy without getting warnings.

@JamesHutchison
Copy link

JamesHutchison commented Nov 10, 2023

Alright, I did some research then consulted with GPT-4 and it's voting snake_case. Here's that discussion:

https://chat.openai.com/share/3788cfb9-d017-4659-bbc2-9476400c3e25

I don't feel strongly about this. It's worth calling out that GPT-4 suggested that pascal vs snake case was a differentiator in tsx / jsx files but I don't see any clear technical reason. I think based on what I know now, the question is whether pyx files may exist in the future and if we want to see this:

<MyComponent foo="1" />

or this:

<my_component foo="1" />

Aesthetically, PascalCase feels better IMO.

GPT-4 seems to insist the naming convention is important in React. Again, I'm not sure there's any real technical reason for a theoretical pyx to strictly use one over the other. We're not sure pyx should even exist.

If you use lowercase for a component name, React will interpret it as a DOM tag. For example, if you have a component named myComponent (with a lowercase 'm') and try to use it in JSX like , React will look for an HTML tag named myComponent, which does not exist, and you will likely get an error or unexpected behavior.

Here's what would happen:

React will treat as a regular HTML tag because it's lowercase.
Since there's no standard HTML tag called myComponent, React will not render anything to the DOM for this tag, or it may throw an error in the console indicating that the tag is not recognized.
The correct way is always to capitalize custom component names, like MyComponent, to ensure that React treats them as components rather than HTML tags.

One final side effect to consider is whether strictly following pep8 and existing linters would drive people to use class based components which are more verbose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority-3-low May be resolved one any timeline. type-docs About changes and updates to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants