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

Documentation: Coding Guidelines: Prescribe specific camelCasing behaviors #7670

Merged
merged 3 commits into from
Jul 9, 2018

Conversation

aduth
Copy link
Member

@aduth aduth commented Jul 2, 2018

Related: #6741, #2511

This pull request seeks to expand upon variable naming guidelines, making it clear that WordPress' camelCasing guidelines are to be followed, with more specific treatment in Gutenberg's handling of abbreviations, acronyms, constants, and the ES2015 class construct.

Testing instructions:

There is no impact on the application.

@aduth aduth added [Type] Developer Documentation Documentation for developers [Type] Code Quality Issues or PRs that relate to code quality labels Jul 2, 2018

#### Class Definition

A [`class` definition](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes) must use the UpperCamelCase convention, regardless of whether it is intended to be used with `new` construction.
Copy link
Member

Choose a reason for hiding this comment

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

Should we note that React function components should be UpperCamelCase as well?

Copy link
Member

Choose a reason for hiding this comment

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

Just noting that we aren't consistent ourselves because we use edit and save for blocks where we should really be using Edit and Save...

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a tricky one. I think the functional components should be UpperCamelCase, yes. Part of that is an expectation that it could be converted to a class component at any point. And also just for consistency. edit and save are interesting to note. They were originally meant to be "just functions", not strictly so much a component: save at one time could even just return a markup string. When we extract the component proper, we do name them "correctly" (e.g. ImageEdit). And in general a capitalized property name seems a bit strange, though I think at one point we were doing the destructure as const { edit: Edit } = blockType;. Maybe it's enough to make that distinction on declaration of variable / class, vs. property name?

Copy link
Member

Choose a reason for hiding this comment

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

I think at one point we were doing the destructure as const { edit: Edit } = blockType;. Maybe it's enough to make that distinction on declaration of variable / class, vs. property name?

Yes, it makes sense. In addition we can change the way we import edit for core blocks to work as you described. I can do it as a follow-up as I introduced this shortcut 😓

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Nice addition 💯

@aduth
Copy link
Member Author

aduth commented Jul 3, 2018

Will plan to let this sit until after the Core JS meeting today, just in case any last-minute objections are raised.

Copy link
Member

@ntwb ntwb left a comment

Choose a reason for hiding this comment

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

👍 👍

@aduth
Copy link
Member Author

aduth commented Jul 3, 2018

From today's Core JS meeting, I'm going to make some clarifications to beginning-of-variable naming with respect to acronyms, e.g. both let domDocument; and class DOMDocument are the expected forms and should be detailed in the guidelines as appropriate to prescribe this.

@gziolo gziolo mentioned this pull request Jul 5, 2018
3 tasks
@aduth
Copy link
Member Author

aduth commented Jul 9, 2018

I added start-of-variable abbreviation naming clarification and components clarification in 8531576 and 4263e55 respectively.

@aduth
Copy link
Member Author

aduth commented Jul 9, 2018

Considering those last two additions as already having been agreed upon. Particulars of phrasing can be iterated upon in subsequent pull requests if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Code Quality Issues or PRs that relate to code quality [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants