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

chore: update eslint #3356

Merged
merged 18 commits into from
Sep 27, 2022
Merged

chore: update eslint #3356

merged 18 commits into from
Sep 27, 2022

Conversation

jaworek
Copy link
Contributor

@jaworek jaworek commented Sep 6, 2022

Eslint config of RN Paper is quite outdated. As a part of testing latest @callstack/eslint-config, I updated it.

Summary

Update eslint config, eslint, and plugins that are used.

Test plan

Pipelines pass and there are no regressions in functionality.

@callstack-bot
Copy link

callstack-bot commented Sep 6, 2022

Hey @jaworek, thank you for your pull request 🤗. The documentation from this branch can be viewed here.

@github-actions
Copy link

github-actions bot commented Sep 6, 2022

The mobile version of example app from this branch is ready! You can see it here

.

3 similar comments
@github-actions
Copy link

github-actions bot commented Sep 6, 2022

The mobile version of example app from this branch is ready! You can see it here

.

@github-actions
Copy link

github-actions bot commented Sep 6, 2022

The mobile version of example app from this branch is ready! You can see it here

.

@github-actions
Copy link

github-actions bot commented Sep 6, 2022

The mobile version of example app from this branch is ready! You can see it here

.

@lukewalczak lukewalczak added the dependencies Pull requests that update a dependency file label Sep 6, 2022
@github-actions
Copy link

github-actions bot commented Sep 6, 2022

The mobile version of example app from this branch is ready! You can see it here

.

package.json Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
package.json Show resolved Hide resolved
.eslintrc Show resolved Hide resolved
.eslintrc Outdated Show resolved Hide resolved
.eslintrc Outdated
"react-native/no-raw-text": "off",
"react-native-a11y/has-accessibility-hint": "off",
// crashes for some reason with error: TypeError: Cannot read properties of undefined (reading 'type')
"react-native-a11y/has-valid-accessibility-state": "off"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

accessibilityState={{ ...accessibilityState, disabled }} in FAB.tsx and AnimatedFAB.tsx causes this rule to crash. If I extract creation of this object to variable, e.g.

const newAccessibilityState= { ...accessibilityState, disabled };

and then use it in component this way:

accessibilityState={newAccessibilityState}

It does not crash anymore. @thymikee, @lukewalczak, do you think this is a bug with this rule and should be reported to react-native-a11y repo?

Copy link
Member

Choose a reason for hiding this comment

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

File a bug in react-native-a11y

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

@jaworek
Copy link
Contributor Author

jaworek commented Sep 12, 2022

After updating to newest callstack eslint config, I'm getting error that plugins are not present:
image

I was able to resolve it by recreating yarn.lock with new package versions. Unfortunately this causes around 18 errors in TS, that I haven't been able to resolve yet. :(

Here's the TS error output:
https://app.circleci.com/pipelines/github/callstack/react-native-paper/2736/workflows/5466e829-16e6-4bda-870a-5f8e79198a34/jobs/30088/parallel-runs/0/steps/0-105

@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

Copy link
Contributor

@Drakeoon Drakeoon left a comment

Choose a reason for hiding this comment

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

@jaworek Good stuff! I left a couple of comments in my review

docs/pages/src/Showcase.js Outdated Show resolved Hide resolved
example/src/DrawerItems.tsx Outdated Show resolved Hide resolved
example/src/Examples/BannerExample.tsx Outdated Show resolved Hide resolved
example/src/Examples/BottomNavigationExample.tsx Outdated Show resolved Hide resolved
@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

@lukewalczak
Copy link
Member

Thanks @jaworek and @Drakeoon for updating the eslint! 🔥

@lukewalczak lukewalczak merged commit 35dc036 into main Sep 27, 2022
@lukewalczak lukewalczak deleted the eslint-update branch September 27, 2022 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants