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

Settle on file naming convention throughout the codebase #3771

Closed
thymikee opened this issue Jun 8, 2017 · 11 comments
Closed

Settle on file naming convention throughout the codebase #3771

thymikee opened this issue Jun 8, 2017 · 11 comments

Comments

@thymikee
Copy link
Collaborator

thymikee commented Jun 8, 2017

Do you want to request a feature or report a bug?
Refactor

What is the current behavior?
Files are named pretty inconsistenly, which makes me sad.

What is the expected behavior?
File naming is consistent, using camelCase or kebab-case (I'm on camelCase train)

I think it would be pretty nice bootcamp task for new Jest contributors, once we settle on the proper naming :)

@aaronabramov
Copy link
Contributor

aaronabramov commented Jun 8, 2017

i like undercores + everything lower case (dependency_resolver.js) because:

  • macOS is case insensitive, and case issues happen quite often ("works locally, breaks on CI", or "i renamed the file, but it's already added to git and git won't see my new name")
  • there's no convention on when to capitalize the first character (some of them are 'exporting a class, an object that has multiple functions, a constants object, etc.'), but nothing concrete and i've never seen it being consistent.
  • consistent with other namings (node_modules, __tests__, __mocks__)

using camel case tho saves 1 character everywhere you'd type an underscore, and it looks more like what you'd name a variable in JS.

i know a lot of people dislike using underscores in file names, but i never heard any reasonable explanation why, so i'd be curious to go through pros/cons of each convention :)

@aaronabramov
Copy link
Contributor

i think this file tree looks pretty consistent and it's very easy to make a decision on naming new files:

__mocks__/
  my_file.js
__tests__/
    my_file.test.js
    my_class.test.js
my_file.js
my_class.js
node_modules/
    ...

@thymikee
Copy link
Collaborator Author

thymikee commented Jun 8, 2017

Actually after @DmitriiAbramov I'm less interested in camel case naming.
So I guess it's mostly to decide if we go underscore_case or kebab-case.

@anilreddykatta
Copy link
Contributor

@thymikee Do we want to go with underscore case. After reading @DmitriiAbramov comments, I liked it would be ideal. If you agree I can raise a PR with changes to file names across repo?

@aaronabramov
Copy link
Contributor

i think we're good to go with underscores. I had a quick chat with @cpojer and it seems like he doesn't mind either.

he mentioned that npm package names usually have - in their names, but i think that's ok, since it's not consistent either and a lot of packages have _ in theri names as well. (also internal node modules like child_process and string_decoder use _)
also i think having --tests-- and --mocks-- would be super weird :)

@aaronabramov
Copy link
Contributor

and yeah, it would be awesome if you could work on it @anilreddykatta!

@pedrottimark
Copy link
Contributor

Thank you for recommending underscores. An unstated assumption is naming convention applies to descendants of src directories, not renaming babel-jest and so on under packages, also leaving alone names like flow-typed which is implicit assumption of Flow, I think?

How does this apply to -test.js convention for the tests in Jest packages? Change to .test.js?

What do you think about a gray areas like names of pretty-format plugins? Having file name like ReactElement.js correspond to imported name ReactElement seems like an exception worth considering?

Last, how hard will it be to rebase any open PRs that get caught in the middle of this?

@aaronabramov
Copy link
Contributor

aaronabramov commented Jun 13, 2017

@pedrottimark I'd really want to have as few exceptions as possible, so that there's never any confusion around "how do i name my file?" question. (ideally this question should not exist at all).

moving to .test.js sounds reasonable to me, since it's already the default and our getting started example is using it.

i think the only exception that we'd have is naming publicly exported packages/files (like jest-*', 'pretty-format, etc), or anything that is required to be named in a certain way (npm-debug.json?)

for react components. i think it's still worth naming them as react_element.js. I agree that writing const ReactElement = require('ReactElement'); feels right, but i don't see any other reasons to keep it this way except my personal preference. If we do keep it as ReactElement.js we get into the same kind if problems (case sensitivity or.. we introduce another file format, should we use style 1 or style 2 for it?)

@aaronabramov
Copy link
Contributor

@pedrottimark as for rebase, i think github is pretty smart with renaming and can figure out that it's the same file with a different name. but i can't promise that :)

@aaronabramov
Copy link
Contributor

followup task #3890

aaronabramov pushed a commit that referenced this issue Jun 29, 2017
* Resolved conflicts

* Including eslint file for forcing file names

* renaming files

* renaming files
tushardhole pushed a commit to tushardhole/jest that referenced this issue Aug 21, 2017
* Resolved conflicts

* Including eslint file for forcing file names

* renaming files

* renaming files
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants