Skip to content

Conversation

@hypest
Copy link
Contributor

@hypest hypest commented Apr 30, 2018

Using the latest from GB, including WordPress/gutenberg#5816.

Main changes include a minor file structure change (blocks library now provided by @wordpress/blocks) and the update from GB master.

This PR drops Prettier in favor of the GB provided eslint config. Unfortunately, the two (Calypso Prettier, GB's eslint) have become incompatible. EDIT: integrated the formatting part by using prettier-eslint.

hypest added 8 commits April 30, 2018 11:27
Calypso Prettier (the fork used so far) has become incompatible with the
eslint config in GB. GB needs `arrowParens: always` but the Calypso fork
doesn't allow to deviate from its default config which is `avoid`. So,
dropping Prettier until the conflict is resolved.
@hypest
Copy link
Contributor Author

hypest commented Apr 30, 2018

@jsnajdr gave a headsup about a PR that updates the Calypso Prettier fork to allow for using a config file. Will git it a try here.

EDIT: gave github:Automattic/calypso-prettier#wp-prettier-1.12.1" a try but it currently doesn't support adding paren spaces in the Flow function type annotations and eslint's setup expects it there too. So, for the time being, the PR stands as is, dropping prettier.

@maxme
Copy link
Contributor

maxme commented May 2, 2018

This PR drops Prettier in favor of the GB provided eslint config. Unfortunately, the two (Calypso Prettier, GB's eslint) have become incompatible.

Noooo!!!

hypest added 2 commits May 3, 2018 14:36
Passing through Prettier+ESLint for formatting but only ESLint for
linting/checking.
@hypest
Copy link
Contributor Author

hypest commented May 3, 2018

Found some middle ground with the prettier-eslint package: using ESLint+Prettier for formatting but only ESLint for checking. See 7e3a19a.

This way VSCode can be used for format-on-save as usual.

@hypest
Copy link
Contributor Author

hypest commented May 7, 2018

With 1e98ed1, the project depends on "@wordpress/is-shallow-equal": "^1.0.1" but that version has a known bug that prevents Metro from bundling the app. Need to wait for WordPress/packages#124 to be merged first and we should update the dependency.

@hypest hypest force-pushed the feature/updated-from-gb-master-with-more branch 2 times, most recently from 43fa3fb to a792f56 Compare May 7, 2018 22:49
@hypest hypest force-pushed the feature/updated-from-gb-master-with-more branch from a792f56 to 6972a55 Compare May 8, 2018 13:34
@hypest
Copy link
Contributor Author

hypest commented May 8, 2018

@wordpress/is-shallow-equal got fixed so, lifting the not ready for review flag.

@hypest
Copy link
Contributor Author

hypest commented May 24, 2018

We're short on RN review capacity atm so, merging this as more work depends on it.

@hypest hypest merged commit 1506cb4 into master May 24, 2018
@hypest hypest deleted the feature/updated-from-gb-master-with-more branch May 24, 2018 12:59
@jsnajdr
Copy link

jsnajdr commented Jun 13, 2018

@hypest is this the incorrect Prettier formatting of Flow type annotations you mentioned in #43 (comment)?

 import ActionTypes from './ActionTypes';
 
-export type BlockActionType = string => {
+export type BlockActionType = (string) => {
        type: $Values<typeof ActionTypes.BLOCK>,
        uid: string,
 };

Also, what about the type parameters wrapped inside <> brackets? Wouldn't spaces around them be more consistent with our style?

Values< typeof ActionTypes.BLOCK >

I know very little about Flow at the moment, so I don't have any opinion on that.

@hypest
Copy link
Contributor Author

hypest commented Jun 18, 2018

Hey @jsnajdr 👋 ,

is this the incorrect Prettier formatting of Flow type annotations you mentioned in #43 (comment)?

Yes, the issue was that the Prettier fork demanded that the non-empty arrow function's parenthesis are removed while the GB Eslint config demanded exactly the opposite.

Also, what about the type parameters wrapped inside <> brackets? Wouldn't spaces around them be more consistent with our style?

Hmm, not sure at this point. Is that a blocker for the work related to the arrow-parens rule perhaps? If a blocker let's add the spaces as I don't think it will be a problem for flow either way.

@jsnajdr
Copy link

jsnajdr commented Jun 22, 2018

@hypest If you're interested in experimenting with the latest Prettier version, the wp-prettier-master branch in the forked repo has some basic support for Flow. It should solve the issues you found while working on gutenberg-mobile, although it's still a long way to have a complete and bug-free support for Flow.

@hypest
Copy link
Contributor Author

hypest commented Jul 4, 2018

👋 @jsnajdr , haven't forgotten you :). Just haven't got the time to try out the wp-prettier-master branch yet. Still on my todo list though :)

hypest pushed a commit that referenced this pull request Nov 2, 2018
Update the sample app to keep a separated size property for each item.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants