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

[RNMobile][Monorepo] Fix linting #21349

Merged

Conversation

ceyhun
Copy link
Member

@ceyhun ceyhun commented Apr 2, 2020

Description

This is a part of the migration of gutenberg-mobile to the gutenberg repo. See #18508

  • Fixed all lint-js errors (mostly with npm run lint-js:fix)
  • Fixed trivial warnings like having /** @format */ or /** @flow */ on top of files
  • There are still some warnings about Illegal usage of jasmine global eslint(jest/no-jasmine-globals) that are not trivial and can be handled on a later stage
  • Removed the remaining flow types

How has this been tested?

  • npm run lint should complete without errors
  • add a console.log to a .js file in packages/react-native-editor/ and npm run lint should return an error Unexpected console statement no-console

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@dratwas dratwas mentioned this pull request Apr 2, 2020
21 tasks
@github-actions
Copy link

github-actions bot commented Apr 2, 2020

Size Change: 0 B

Total Size: 856 kB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 998 B 0 B
build/annotations/index.js 3.43 kB 0 B
build/api-fetch/index.js 3.39 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.02 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/index.js 100 kB 0 B
build/block-editor/style-rtl.css 10.9 kB 0 B
build/block-editor/style.css 10.9 kB 0 B
build/block-library/editor-rtl.css 7.24 kB 0 B
build/block-library/editor.css 7.24 kB 0 B
build/block-library/index.js 110 kB 0 B
build/block-library/style-rtl.css 7.42 kB 0 B
build/block-library/style.css 7.43 kB 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.5 kB 0 B
build/components/index.js 191 kB 0 B
build/components/style-rtl.css 15.8 kB 0 B
build/components/style.css 15.7 kB 0 B
build/compose/index.js 6.21 kB 0 B
build/core-data/index.js 10.6 kB 0 B
build/data-controls/index.js 1.04 kB 0 B
build/data/index.js 8.2 kB 0 B
build/date/index.js 5.37 kB 0 B
build/deprecated/index.js 771 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.06 kB 0 B
build/edit-post/index.js 91.2 kB 0 B
build/edit-post/style-rtl.css 8.47 kB 0 B
build/edit-post/style.css 8.46 kB 0 B
build/edit-site/index.js 5.56 kB 0 B
build/edit-site/style-rtl.css 2.62 kB 0 B
build/edit-site/style.css 2.62 kB 0 B
build/edit-widgets/index.js 4.43 kB 0 B
build/edit-widgets/style-rtl.css 2.58 kB 0 B
build/edit-widgets/style.css 2.58 kB 0 B
build/editor/editor-styles-rtl.css 381 B 0 B
build/editor/editor-styles.css 382 B 0 B
build/editor/index.js 43.8 kB 0 B
build/editor/style-rtl.css 3.97 kB 0 B
build/editor/style.css 3.96 kB 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 6.95 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 1.93 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.49 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.69 kB 0 B
build/list-reusable-blocks/index.js 2.99 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 4.83 kB 0 B
build/notices/index.js 1.58 kB 0 B
build/nux/index.js 3.01 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.54 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 780 B 0 B
build/redux-routine/index.js 2.83 kB 0 B
build/rich-text/index.js 14.4 kB 0 B
build/server-side-render/index.js 2.55 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.01 kB 0 B
build/viewport/index.js 1.61 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.18 kB 0 B

compressed-size-action

@ceyhun ceyhun marked this pull request as ready for review April 2, 2020 16:05
@ceyhun ceyhun requested review from Tug and cameronvoell April 2, 2020 16:05
@Tug
Copy link
Contributor

Tug commented Apr 3, 2020

@ceyhun I'm seeing this error when running npm run lint locally:

[0] ../gutenberg/packages/react-native-editor/__device-tests__/helpers/utils.js
[0]   5:1  error  'wd' should be listed in the project's dependencies. Run 'npm i -S wd' to add it  import/no-extraneous-dependencies
[0]

Any idea?

@ceyhun
Copy link
Member Author

ceyhun commented Apr 3, 2020

@Tug It is correct that you're seeing that error and it should be fixed. I'm not sure why I'm not seeing it. I did a clean run again and I'm still not seeing it.

@Tug
Copy link
Contributor

Tug commented Apr 3, 2020

I know this is out of scope of this PR but I took the opportunity to run the unit tests. The main ones are currently failing with multiple syntax errors around react-native-editor/__device-tests__:

 Summary of all failing tests
 FAIL  packages/react-native-editor/__device-tests__/gutenberg-editor-paragraph.test.js
  ● Test suite failed to run

    SyntaxError: .../gutenberg/packages/react-native-editor/__device-tests__/pages/editor-page.js: Support for the experimental syntax 'classProperties' isn't currently enabled (13:21):

      11 |
      12 | export default class EditorPage {
    > 13 | 	paragraphBlockName = 'Paragraph';

...

It also seems jest is failing to interpret flow code from RN:

 FAIL  packages/react-native-editor/src/index.test.js
  ● Test suite failed to run

    .../gutenberg/node_modules/react-native/Libraries/Utilities/warnOnce.js:15
    const warnedKeys: {[string]: boolean} = {};
          ^^^^^^^^^^

    SyntaxError: Missing initializer in const declaration

      at ScriptTransformer._transformAndBuildScript (node_modules/@jest/transform/build/ScriptTransformer.js:537:17)
      at ScriptTransformer.transform (node_modules/@jest/transform/build/ScriptTransformer.js:579:25)
      at Object.<anonymous> (node_modules/react-native/Libraries/react-native/react-native-implementation.js:14:18)

We'll need to either update the main gutenberg jest config (in test/unit/jest.config.js) to use react-native preset or most probably exclude packages/react-native-* from the main tests

@Tug
Copy link
Contributor

Tug commented Apr 3, 2020

@Tug It is correct that you're seeing that error and it should be fixed. I'm not sure why I'm not seeing it. I did a clean run again and I'm still not seeing it.

I'll keep trying and see if I can fix it on my side 👍

@ceyhun
Copy link
Member Author

ceyhun commented Apr 3, 2020

We'll need to either update the main gutenberg jest config (in test/unit/jest.config.js) to use react-native preset or most probably exclude packages/react-native-* from the main tests

Ignored test files in react-native-* packages folders in main unit tests test/unit/jest.config.js but enabled them in main native tests test/native/jest.config.js.

  • npm run test-unit
  • npm run test-unit:native

should run without errors now, except 1 error in block-serialization-spec-parser/test/index.js and 1 in storybook/test/index.js but I think those are not related to the changes here.

@@ -30,6 +30,7 @@ module.exports = {
'<rootDir>/.*/build/',
'<rootDir>/.*/build-module/',
'<rootDir>/.+.native.js$',
'packages/react-native-*',
Copy link
Contributor

Choose a reason for hiding this comment

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

Really minor but we could try to be a bit more consistent with /packages/react-native-*

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated 👍

Copy link
Contributor

@Tug Tug left a comment

Choose a reason for hiding this comment

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

LGTM 👍

As usual, let's have @hypest merge with a merge commit

@hypest
Copy link
Contributor

hypest commented Apr 6, 2020

Will merge with "Allow merge commits" ON and then disable it again.

@hypest hypest merged commit 14258b8 into feat/import-gutenberg-mobile-no-squash Apr 6, 2020
@hypest hypest deleted the rnmobile/monorepo-fix-linting branch April 6, 2020 08:45
@Tug Tug mentioned this pull request Jun 5, 2020
6 tasks
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.

3 participants