Skip to content
This repository has been archived by the owner on Feb 6, 2023. It is now read-only.

Upgrade eslint + flow #2083

Closed
wants to merge 12 commits into from
Closed

Conversation

lukyth
Copy link
Contributor

@lukyth lukyth commented May 23, 2019

Summary

I want to upgrade ESLint and Flow on this project. I ended up with

  • Remove unused eslint-plugin-relay

  • Upgrade all ESLint packages to the latest version and fix all the new lint errors afterward.

    • eslint-config-fbjs
    • eslint-config-prettier
    • eslint-plugin-babel
    • eslint-plugin-flowtype
    • eslint-plugin-jsx-a11y
    • eslint-plugin-prettier
    • eslint-plugin-react
  • Upgrade Flow (flow-bin) to the 0.115.0.

Then I found that yarn test is failing at

src/component/handlers/composition/__tests__/DraftEditorCompostionHandler-test.js: Support for the experimental syntax 'nullishCoalescingOperator' isn't currently enabled (70:40)

So I added @babel/plugin-proposal-nullish-coalescing-operator to jest preprocessor as well.
Move plugins in .babelrc to jest preprocessor as #2083 (comment)

Test Plan

yarn lint, yarn flow, and yarn test should all pass.

@claudiopro
Copy link
Contributor

Hi @lukyth, thanks for contributing a fix to Draft.js! 🎉 Unfortunately I had already started working on this change with #2076 a few days earlier, but I like your change to add the babel plugin to the Jest preprocessor better:

  process(src, filename) {
    var options = {
      presets: [fbjsConfigurePreset({rewriteModules: {map: moduleMap}})],
+     plugins: [require('@babel/plugin-proposal-nullish-coalescing-operator')],
      filename: filename,
      retainLines: true,
    };

whereas I added a .babelrc file. Do you mind rebasing and proposing this change again? I'd really love to get this added and give you credit in the commit history.

@lukyth
Copy link
Contributor Author

lukyth commented Jun 14, 2019

Hi @claudiopro :) Thank you for your comments! I rebased the branch against master and brought back the type coercion. I also took this change to upgrade eslint-plugin-jsx-a11y, eslint-plugin-prettier, and eslint-plugin-react as well. Please have a look again 🙏

I tried to upgrade flow-bin pass version 0.98.1 but it will cause an error from node_modules. facebook/fbjs#346 need to be released first in order to upgrade to a newer version than 0.98.1.

Copy link
Contributor

@claudiopro claudiopro left a comment

Choose a reason for hiding this comment

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

Please take a look at my comments. Since you're configuring the babel plugin @babel/plugin-proposal-nullish-coalescing-operator in the Jest preprocessor, you should remove the .babelrc file which is not necessary anymore.

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
src/.flowconfig Outdated Show resolved Hide resolved
src/model/modifier/__tests__/AtomicBlockUtils-test.js Outdated Show resolved Hide resolved
.eslintrc.js Outdated Show resolved Hide resolved
@lukyth
Copy link
Contributor Author

lukyth commented Jun 14, 2019

@claudiopro I took care of @babel/plugin-proposal-nullish-coalescing-operator 😄, but upgrading flow-bin to a version newer than 0.98.1 will requires facebook/fbjs#346 to be released first 😞.

@mrkev
Copy link
Contributor

mrkev commented Jan 14, 2020

Cool stuff! I want to see what the internal linters say about this.

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@mrkev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@lukyth
Copy link
Contributor Author

lukyth commented Jan 15, 2020

@mrkev Thanks for checking it out :)

@lukyth lukyth changed the title Upgrade eslint + flow, and fix failed test from nullishCoalescingOperator Upgrade eslint + flow Jan 15, 2020
@mrkev
Copy link
Contributor

mrkev commented Jan 15, 2020

There's a linter warning, but it should be fine. I'll try to get this merged today.

@facebook-github-bot
Copy link

@mrkev merged this pull request in 824fd12.

@lukyth lukyth deleted the lukyth/chore branch January 17, 2020 15:30
mmissey pushed a commit to mmissey/draft-js that referenced this pull request Mar 24, 2020
…ator (facebookarchive#2083)

Summary:
**Summary**

I want to upgrade ESLint and Flow on this project. I ended up with
- Remove unused `eslint-plugin-relay`
- Upgrade all ESLint packages to the latest version and fix all the new lint errors afterward.
  - `eslint-config-fbjs`
  - `eslint-config-prettier`
  - `eslint-plugin-babel`
  - `eslint-plugin-flowtype`
  - `eslint-plugin-jsx-a11y`
  - `eslint-plugin-prettier`
  - `eslint-plugin-react`

- Upgrade Flow (`flow-bin`) to the `0.115.0`.

~Then I found that `yarn test` is failing at~
```
src/component/handlers/composition/__tests__/DraftEditorCompostionHandler-test.js: Support for the experimental syntax 'nullishCoalescingOperator' isn't currently enabled (70:40)
```
~So I added `babel/plugin-proposal-nullish-coalescing-operator` to jest preprocessor as well.~
Move plugins in `.babelrc` to jest preprocessor as facebookarchive#2083 (comment)

**Test Plan**

`yarn lint`, `yarn flow`, and `yarn test` should all pass.
Pull Request resolved: facebookarchive#2083

Reviewed By: claudiopro

Differential Revision: D19398251

Pulled By: mrkev

fbshipit-source-id: 4fc2f6dd6e0cd44266d061455b7b6a433231715a
vilemj-Viclick pushed a commit to kontent-ai/draft-js that referenced this pull request Jul 16, 2020
…ator (facebookarchive#2083)

Summary:
**Summary**

I want to upgrade ESLint and Flow on this project. I ended up with
- Remove unused `eslint-plugin-relay`
- Upgrade all ESLint packages to the latest version and fix all the new lint errors afterward.
  - `eslint-config-fbjs`
  - `eslint-config-prettier`
  - `eslint-plugin-babel`
  - `eslint-plugin-flowtype`
  - `eslint-plugin-jsx-a11y`
  - `eslint-plugin-prettier`
  - `eslint-plugin-react`

- Upgrade Flow (`flow-bin`) to the `0.115.0`.

~Then I found that `yarn test` is failing at~
```
src/component/handlers/composition/__tests__/DraftEditorCompostionHandler-test.js: Support for the experimental syntax 'nullishCoalescingOperator' isn't currently enabled (70:40)
```
~So I added `babel/plugin-proposal-nullish-coalescing-operator` to jest preprocessor as well.~
Move plugins in `.babelrc` to jest preprocessor as facebookarchive#2083 (comment)

**Test Plan**

`yarn lint`, `yarn flow`, and `yarn test` should all pass.
Pull Request resolved: facebookarchive#2083

Reviewed By: claudiopro

Differential Revision: D19398251

Pulled By: mrkev

fbshipit-source-id: 4fc2f6dd6e0cd44266d061455b7b6a433231715a
alicayan008 pushed a commit to alicayan008/draft-js that referenced this pull request Jul 4, 2023
…ator (#2083)

Summary:
**Summary**

I want to upgrade ESLint and Flow on this project. I ended up with
- Remove unused `eslint-plugin-relay`
- Upgrade all ESLint packages to the latest version and fix all the new lint errors afterward.
  - `eslint-config-fbjs`
  - `eslint-config-prettier`
  - `eslint-plugin-babel`
  - `eslint-plugin-flowtype`
  - `eslint-plugin-jsx-a11y`
  - `eslint-plugin-prettier`
  - `eslint-plugin-react`

- Upgrade Flow (`flow-bin`) to the `0.115.0`.

~Then I found that `yarn test` is failing at~
```
src/component/handlers/composition/__tests__/DraftEditorCompostionHandler-test.js: Support for the experimental syntax 'nullishCoalescingOperator' isn't currently enabled (70:40)
```
~So I added `babel/plugin-proposal-nullish-coalescing-operator` to jest preprocessor as well.~
Move plugins in `.babelrc` to jest preprocessor as facebookarchive/draft-js#2083 (comment)

**Test Plan**

`yarn lint`, `yarn flow`, and `yarn test` should all pass.
Pull Request resolved: facebookarchive/draft-js#2083

Reviewed By: claudiopro

Differential Revision: D19398251

Pulled By: mrkev

fbshipit-source-id: 4fc2f6dd6e0cd44266d061455b7b6a433231715a
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants