Skip to content

Conversation

@aickin
Copy link
Contributor

@aickin aickin commented Jun 25, 2017

Throughout my work on #10024, I found that I would get intermittent prettier errors from CI, and some of them seemed arbitrary. Changes that didn't fix any prettier issues would make CI prettier stop complaining, and changes that didn't add any bad formatting would make CI prettier get unhappy. Further, npm run prettier or yarn prettier (the recommended fix) never fixed the CI problems I was having.

I've since realized that the reason that npm run prettier wasn't working was that that command attempts to run prettier over only the changed files, but CI checks all files to make sure they are correctly formatted. And, for whatever reason, master has a few dozen files that are incorrectly formatted right now. I suspect these files creeped in because of some problem in CI that was causing the arbitrariness I was seeing, but I'm not entirely sure.

This PR is just a result of running npm run prettier-all over the codebase and then fixing one tiny problem with linting (prettier eliminated three eslint-disable-line comments in ReactDOMServerIntegration-test.js which I had to reinstate).

This PR doesn't fix the core problem that CI seems to sometimes approve of builds that violate prettier's formatting and thereby lets incorrectly formatted files in, but I honestly can't really figure out why that happens. In the meantime, though, I'm hoping that this checkin will reduce the number of times CI complains about my other PRs.

@aickin
Copy link
Contributor Author

aickin commented Jun 25, 2017

Appropriately, this rejected by CI because of prettier... <sigh>.

On the bright side, I think I've actually figured out what's going on here!

I believe the problem is that the prettier version in package.json is ^1.2.2. Because of this, anything from 1.2.2 to 1.4.0 (the current release) is allowed. However, the code in master is formatted by prettier 1.2.2, which that creates a different output than prettier 1.4.0 (the version I used in this PR). My theory is that some of the CI machines have prettier 1.2.2 cached, while others are getting prettier 1.4.0, so whether or not the build passes the prettier step depends on which build machine you get. This certainly lines up with my experience of the build.

I'll close this PR and open a simpler PR that just pins the version of prettier to 1.2.2, which is the version the current code expects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants